New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update MultiplayerGameLobby controls visibility logic. #318

Open

frg2089 wants to merge 1 commit into CnCNet:develop from frg2089:feature/new-controls-loader

Contributor

frg2089 commented Mar 26, 2022

This will provide more flexible control visibility support.
Applying different styles depending on the suffix will no longer be limited to "lbChatMessages" and "tbChatInput" controls.

Now we can write INI like this.

[MultiplayerGameLobby]
$CC-ChatMessages=ChatMessages:XNAPanel

[ChatMessages]
$X=1
$Y=1
$CC-lbChatMessages=lbChatMessages:ChatListBox

[ChatMessages_Host]
$BaseSection=ChatMessages
Size=100,200
BackgroundTexture=ChatMessagesBorder_Host.png

[ChatMessages_Player]
$BaseSection=ChatMessages
Size=100,100
BackgroundTexture=ChatMessagesBorder_Player.png

On INItializableWindow Initialized.
ParseExtraControls() method will create an XNAPanel named ChatMessages.

Then the Refresh() method will be called when creating a game or joining a game.
The ChatMessages control will be renamed to ChatMessages_Host or ChatMessages_Player via the IsHost field. and reload its properties.

devo1929 approved these changes

View reviewed changes

github-actions bot commented May 12, 2022 •

edited

Nightly build for this pull request:

artifacts-TS.zip
artifacts-YR.zip
artifacts-Ares.zip
This comment is automatic and is meant to allow guests to get latest automatic builds without registering. It is updated on every successful build.

frg2089 force-pushed the feature/new-controls-loader branch from 1b28b1a to 58f8aa0 Compare

June 11, 2022 02:49

Metadorius requested review from Rampastring and Starkku

June 16, 2022 18:59

frg2089 force-pushed the feature/new-controls-loader branch from 58f8aa0 to c6a54ac Compare

June 17, 2022 08:34

frg2089 force-pushed the feature/new-controls-loader branch from c6a54ac to 8bccb29 Compare

September 21, 2022 23:16

Metadorius requested changes

View reviewed changes

Member

Metadorius left a comment

Couple of nitpicks. I am not quite sure this is the direction we want to take (with hardcoded suffixes and handling only the very special case of map list visibility), I would prefer a more generalized solution. But the execution of that idea is more or less fine.

@Rampastring what's your opinion? Basically the code is very specific to this case and introduces INI behavior based on section names, which is only tied to map box being shown or not.

DXMainClient/DXGUI/Multiplayer/GameLobby/MultiplayerGameLobby.cs Outdated

Comment on lines 643 to 694

+                              ddGameModeMapFilter.Enable();
+                              lblGameModeSelect.Enable();
+                              lbGameModeMapList.Enable();
+                              tbMapSearch.Enable();
+                              btnPickRandomMap.Enable();

Member

Metadorius Nov 29, 2022

Pretty sure you forgot btnMapSortAlphabetically. Also wouldn't tunnel selection button need to be handled too at CnCNet game lobby? Maybe would be good to have this function as protected virtual?

DXMainClient/DXGUI/Multiplayer/GameLobby/MultiplayerGameLobby.cs Outdated Show resolved Hide resolved

DXMainClient/DXGUI/Multiplayer/GameLobby/MultiplayerGameLobby.cs Outdated

Comment on lines 640 to 641

		sourceSuffix = "_Player";
		targetSuffix = "_Host";

Member

Metadorius Nov 29, 2022

You should make those consts at class level.

DXMainClient/DXGUI/Multiplayer/GameLobby/MultiplayerGameLobby.cs Outdated Show resolved Hide resolved

DXMainClient/DXGUI/Multiplayer/GameLobby/MultiplayerGameLobby.cs Outdated

+                      /// Change MapList and other controls visibility.
+                      /// </summary>
+                      /// <param name="isHost">Visibility</param>
+                      private void ChangeMapListAndOtherControlsVisibility(bool isHost)

Member

Metadorius Nov 29, 2022

Not fond of the name, needs something more concise.

DXMainClient/DXGUI/Multiplayer/GameLobby/MultiplayerGameLobby.cs Outdated

Comment on lines 38 to 40

    
                                  s => ChangeMapListAndOtherControlsVisibility(false)),

                              new ChatBoxCommand("SHOWMAPS", "Show map list (game host only)".L10N("UI:Main:ChatboxCommandShowMapsHelp"), true,

                                  s => ShowMapList()),

                                  s => ChangeMapListAndOtherControlsVisibility(true)),

Member

Metadorius Nov 29, 2022

No, you're mixing two things. The prefixes you have and the bool you use as a parameter suggest differentiation between host and player, yet you're mixing this with map list hidden or shown.

frg2089 force-pushed the feature/new-controls-loader branch 2 times, most recently from f9862e5 to 3bb9360 Compare

November 30, 2022 04:04

frg2089 requested review from Metadorius and removed request for Starkku and Rampastring

November 30, 2022 04:04

frg2089 closed this

frg2089 deleted the feature/new-controls-loader branch

February 22, 2024 00:16

frg2089 restored the feature/new-controls-loader branch

February 22, 2024 00:17

frg2089 reopened this

frg2089 force-pushed the feature/new-controls-loader branch 2 times, most recently from f577488 to a2f047d Compare

February 26, 2024 12:48


          Update MultiplayerGameLobby controls visibility logic.

Signed-off-by: 舰队的偶像-岛风酱! <frg2089@outlook.com>

frg2089 force-pushed the feature/new-controls-loader branch from a2f047d to 5866048 Compare

June 2, 2024 10:27

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