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.

r14098

>	ratingList.push(String("  " + rating));

r14762

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);

r17389

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.

Change History (2)

comment:1 by elexis, 4 years ago

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, or IGUIObject size property.

comment:2 by elexis, 4 years ago

In 23172:

Rewrite lobby page to use class semantics, add more gamedetails labels, improve performance using batch processing and caching and gain possibility for game creation/player-join/leave events, refs #5387.

Game selection details features:

  • Display victory conditions following their sending but missing display following rP14098, refs rP19642.
  • Display the host of the match and the game name in the selected game details following rP19703, refs D1666.
  • Display mods if the mods differ (without having to attempt to join the game prior) following rP21301.

Performance features:

  • Implement batch message processing in the XmppClient to rebuild GUI objects only once when receiving backlog or returning from a match.
  • Implement Game class to cache gamelist, filter and sorting values, as they rarely change but are accessed often.
  • Cache sprintf objects.

Security fixes:

  • Add escapeText in lobby/ to avoid players breaking the lobby for every participant, supersedes D720, comments by bb.
  • Do not hide broadcasted unrecognized chat commands that mods used as leaking private channels, fixes #5615.

Defect fixes:

  • Fix XmppClient.cpp storing unused historic message types resulting in memory waste and unintentional replay of for instance disconnect/announcements messages following both rP20070/D819 and rP22855/D2265, refs #3306.
  • Fix XmppClient.cpp victoryCondition -> victoryConditions gamesetup.js change from rP21474/D1240.
  • Fix leaderboard/profile page cancel hotkey closing the lobby dialog as well and removes cancel hotkey note from lobby_panels.xml from rP20886/D817 since the described issue was fixed by rP22200/D1701.
  • Fix lobby playing menu sound in a running game after having closed the lobby dialog following introduction in rP20886/D817.
  • Fix GUI on nick change by updating g_Username.
  • Update profile panel only with data matching the player requested.

Hack erasure:

  • Object semantics make it cheap to add state and cache values, storing literals in properties while removing globals, adding events while decoupling components and gaining moddability.
  • Erase comments and translation comments stating that this would be IRC!!, supersedes D1136.
  • Introduce Status chat message type to supersede "/special" chat command + "isSpecial" property from rP14098 (formerly g_specialKey rP17360) deluxe hack.
  • Introduce System chat message type to supersede system errors disguising as chat from a mock user called "system".

Code cleanups:

  • Move code from XML to JS.
  • Move size values from JS to XML, especially following rP20886/D817 and rP21003/D1051.
  • Rename "user" to "player".
  • Fix lobby/ eslint warnings, refs D2261.
  • Remove message.nick emptiness check from rP20064/D835, since XEP-0045 dictates that it is non-empty.
  • Add translated string for deleted subjects.
  • Add TODOs for some evident COList issues, refs #5638.

Differential Revision: https://code.wildfiregames.com/D2412

Note: See TracTickets for help on using tickets.