search

beyond-all-reason / BYAR-Chobby

55 stars 64 forks source link

Replay Menu: Full Playerlist via Tooltip #715

Open robertthepie opened 1 week ago

robertthepie commented 1 week ago

Work Done

Updated the Replay Menu Player List to:

Testing done

The replay menu has been tested on the following team sizes: 1v1, 2v2, 4v4, 5v5, 8v8 20v20 6 teams of 2, 6 teams of 3, 8 teams of 2, 16 player ffa, 11 player ffa, 5 player ffa

FIr3baL commented 6 days ago

Hey @robertthepie , i'll review this PR and give some feedback...

After first launch, i see a lot of unused space, when i use Split Panel Mode = "Always One". See pic. So, i suggest to use that space for the players. Though this can be done in a later PR. No stopper for this PR imo... grafik

robertthepie commented 6 days ago

After first launch, i see a lot of unused space, when i use Split Panel Mode = "Always One".

The old system simply overflowed outside of the rendered panel, which worked great for when it was expanded, or too short, as it didn't account for it at all. Reverted back to that.

FIr3baL commented 4 days ago

I got 2 concerns so far...

  1. playerWidget I was wondering why gui_replay_handler creates its own user-controls. Chobby uses api_user_handler for creating and updating user-controls everywhere. But i understand that this is a special case. We take the values - for now only rank - from the replay-file and don't want them to be updated by multiplayer events. It should display the values from the replay not current live values.

This PR is duplicating the function playerWidget into gui_tooltip. This is not good in terms of maintenance. I'll try to add a ReplayTooltipUser to api_user_handler with parameters for rank and skill. Then we can remove playerWidget from gui_tooltip and gui_replay_handler. (Pretty sure i find the time next 2-3 days)

  1. tooltipText concatenation Since we have already built a structured table with all replay-infos in gui_replay_handler, we should look for a way to pass this table to our tooltip widget.

For battles all these infos are stored in lobby.lua and other widgets can receive infos from this central object.

Maybe gui_replay_handler could provide an externalFunctions:GetReplayData(replayID or replayName). This way we'd only need to add an id to tooltipText and gui_tooltip could access this infos without serializing/deserializung them in between. This should help with readability and later expansion of this part. E.g. adding skill or such to tooltip users.