Opened 4 years ago
Last modified 4 years ago
#5638 new defect
COList: Support column text alginment and modifying individual rows
Reported by: | elexis | Owned by: | |
---|---|---|---|
Priority: | Should Have | Milestone: | Backlog |
Component: | Core engine | Keywords: | |
Cc: | Patch: |
Description
In the course of Phab:D2412 the following missing features of COList.cpp
became evident:
Since the introduction the rating list didn't center the text correctly.
> ratingList.push(String(" " + rating));
Displays ratings on the gamesetup page and formats ratings less than 1000 correctly.
var ratingSpaces = " "; for (var index = 0; index < 4 - Math.ceil(Math.log(cleanPlayerList[i].rating) / Math.LN10); ++index) ratingSpaces += " ";
r17366 Simplifies updatePlayerList
> let rating = (" " + (player.rating ? player.rating : "-")).substr(-5);
Tune lobby rating alignment
let rating = player.rating ? (" " + player.rating).substr(-5) : " -";
As we notice this is just changing one kind of solution that is conceptually broken because it tries to fix a problem that can only be properly solved woth a C++ diff in JS.
Centering text is not an invention, the columns should simply support a way to set a center text alignment.
CGUIText
uses if (pObject->SettingExists("text_align"))
to obtain the alignment.
This makes it impossible to set an alignment for only one column currently, instead the columns should either become objects themselves (inherit IGUIObject or even CGUIText which makes them gain some properties such as text alignment), or COListColumns should receive more attributes that can be changed.
2.
In the performance measurement of Phab:D2412 it became evident that the COList list_foo = bigarray
assignments are comparably slow and can consume something like 2ms each call.
60 FPS means 16ms per frame 100 FPS means 10ms per frame 200 FPS means 5ms per frame
So you can see that cutting out 2ms would remove occasional framedrops when there are many COList replacements performed.
Since the list shouldn't be replaced so often to begin with, the issue doesn't matter much, but it might be good to do it right. As long as the player doesn't change the sorting order, only one entry will be changed.
This is a similar problem to the CList appendText() function proposed in Phab:D1781.
So one should probably consider a design that encompasses
- insertion of a row
- deletion of a row
- change of a row
This would mean 3 JS functions which sounds ugly, array operations might be more familiar and more functionally rich.
(If the JS Array was not recreated everytime but rooted and only modified in JS, then array operations such as splice
might be used on it to cover all these 3 use cases and array functions?
It would probably have to be a JSClass and not an array, since one can't track which row of the table changed otherwise?)
Aside from the lobby where playerlists are updated very frequently, the replay menu also had a use case in deleting individual rows (when deleting replay files), and there may be hundreds and hundreds of replays for some players.
"Support column text alginment" is "should have" since the existing code is a bad workaround. "modifying individual rows" seem "nice to have" since COList list replacements should not happen so often to begin with.
3rd TODO:
Some columns are optional (can be hidden). For example the rating / maptype column are optional in the lobby. But there are also filters GUI objects positioned in alignment with the COList columns. It should be possible to obtain the position of the columns, similar to
CText
getTextSize
, orIGUIObject
size
property.