Skip to content

feat: ControlsSettingsTable module#7264

Draft
SobakaPirat wants to merge 5 commits intoLiquipedia:mainfrom
SobakaPirat:ControlsSettingsTable
Draft

feat: ControlsSettingsTable module#7264
SobakaPirat wants to merge 5 commits intoLiquipedia:mainfrom
SobakaPirat:ControlsSettingsTable

Conversation

@SobakaPirat
Copy link
Contributor

@SobakaPirat SobakaPirat commented Mar 17, 2026

Summary

Module for saving and displaying player's input settings, based on current trackmania and rocket league versions, with a focus on making the table columns as flexible as possible for any wiki.

How did you test this change?

Custom module example for trackmania:
https://liquipedia.net/trackmania/Module:ControlsSettingsTable/Custom/dev/soba2
https://liquipedia.net/trackmania/User:SobakaPirat/ControlsSetingsTable

изображение

@SobakaPirat SobakaPirat requested review from a team as code owners March 17, 2026 17:09
Copy link
Collaborator

@hjpalpha hjpalpha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • is not designed
  • uses old table styles
  • missing annos in some places
  • commented out code in several places
  • entire custom seems useless
  • expands templates
  • calls parsers
  • mixes single and double quotes
  • uses stringified html instead of widgets
  • uses lots of parsing that should use the libs (e.g. DateExt) we have instead


---@param args {[string]: string?}
---@return ColumnConfig[]
local function makeColumnConfig(args)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't depend on args
this is just a copy ...
seems like a useless function

---@alias ColumnConfig
---| {key: string, title: string}
---| {keys: ({key: string} | string)[], title: string}
---@alias ColumnValue {title: string, value: fun(data: {[string]: string?}): string?}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these aliases are bad (way too generic)

Copy link
Collaborator

@ElectricalBoy ElectricalBoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this really be placed in Commons?

@hjpalpha
Copy link
Collaborator

should this really be placed in Commons?

fwiw i am not opposed to having a well-designed clean base version on commons but i don't think this is it

@SobakaPirat SobakaPirat marked this pull request as draft March 18, 2026 08:34
@SobakaPirat
Copy link
Contributor Author

@hjpalpha I went through your list and completely changed the code
The only thing, Idk if it's possible to make a 'reference' without template

@SobakaPirat SobakaPirat requested a review from hjpalpha March 19, 2026 16:13
Copy link
Collaborator

@hjpalpha hjpalpha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heavily oppose this structure
it completely violates the widget setup

still

  • is not designed
  • uses old table styles / doesn't use table2
  • expands template
  • uses stringified html instead of widgets / builds images as wiki code instead of using widgets
    uses lots of parsing that should use the libs we have instead

arrow_left = 'Gamepad_button_arrow_left.svg',
arrow_right = 'Gamepad_button_arrow_right.svg'
},
} No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eof empty line missing

@SobakaPirat SobakaPirat requested a review from hjpalpha March 24, 2026 16:48
Copy link
Collaborator

@hjpalpha hjpalpha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

additionally
lots of functions that should be private are not
lots of indent issues
should split display into a widget and processing/storage into a differe3nt one
not a fan of the current structure

@@ -0,0 +1,59 @@
---
-- @Liquipedia
-- page=Module:Links/ButtonTranslation
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't get why this is under Links
those are svg icons not links

---@return Widget
function ControlsSettingsTable.create(frame)
local args = Arguments.getArgs(frame)
local config = Info.controlsSettingsTable
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not defined...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants