Skip to content
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.

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

Show map preview in game preview #611

Merged
merged 13 commits into from
Jan 5, 2025

Conversation

GrantBartlett
Copy link
Member

@GrantBartlett GrantBartlett commented Jan 3, 2025

Preview

2025-01-04.13-30-20.mp4

Copy link

github-actions bot commented Jan 3, 2025

Nightly build for this pull request:

@SadPencil SadPencil marked this pull request as draft January 3, 2025 14:50
@GrantBartlett GrantBartlett changed the title Wip map show preview and ping textures Show map preview and ping textures in game preview Jan 4, 2025
@GrantBartlett GrantBartlett marked this pull request as ready for review January 4, 2025 13:34
@GrantBartlett GrantBartlett requested review from SadPencil and Metadorius and removed request for SadPencil January 4, 2025 16:42
@@ -126,9 +176,12 @@ public void SetInfo(GenericHostedGame game)
lblHost.Text = "Host:".L10N("Client:Main:GameInfoHost") + " " + Renderer.GetSafeString(game.HostName, lblHost.FontIndex);
lblHost.Visible = true;

lblPing.Text = game.Ping > 0 ? "Ping:".L10N("Client:Main:GameInfoPing") + " " + game.Ping.ToString() + " ms" : "Ping: Unknown".L10N("Client:Main:GameInfoPingUnknown");
lblPing.Text = "Ping:".L10N("Client: Main: GameInfoPing");
Copy link
Member

Choose a reason for hiding this comment

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

The L10N key name does not come with spaces after :

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@@ -84,11 +119,21 @@ public override void Initialize()
lblPlayerNames[(lblPlayerNames.Length / 2) + i] = lblPlayerName2;
}

pingTextures = new Texture2D[5]
{
AssetLoader.LoadTexture("ping0.png"),
Copy link
Member

Choose a reason for hiding this comment

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

Will the client work normally when these assets are missing?

Copy link
Member Author

Choose a reason for hiding this comment

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

These are the same ping images used in the cncnetgamelobby

if (mapLoader != null)
{
mapTexture = mapLoader.GameModeMaps.Find((m) => m.Map.Name == game.Map)?.Map.LoadPreviewTexture();
if (mapTexture == null)
Copy link
Member

Choose a reason for hiding this comment

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

You can use ?? operator if you want.

Copy link
Member

@Metadorius Metadorius left a comment

Choose a reason for hiding this comment

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

not necessarily exhaustive review, currently a bit exhausted to properly look at so may miss somehting

DXMainClient/DXGUI/Multiplayer/GameInformationPanel.cs Outdated Show resolved Hide resolved
Comment on lines 70 to 73
leftColumnPositionX = 10;
rightColumnPositionX = Width / 2 - 10;
topStartingPositionY = 30;
mapPreviewPositionY = topStartingPositionY + (rowHeight * 2 + 15); // 2 Labels down, incase map name spills to next line
Copy link
Member

Choose a reason for hiding this comment

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

I smell magic numbers here and accross the code

Copy link
Member Author

Choose a reason for hiding this comment

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

Bit of one off padding, doesn't feel too bad

Copy link
Member

Choose a reason for hiding this comment

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

still think it should be easy to get rid of the magic numbers accross the code and make them constants, minor stuff like this accumulates and makes the code less readable

DXMainClient/DXGUI/Multiplayer/GameInformationPanel.cs Outdated Show resolved Hide resolved
DXMainClient/DXGUI/Multiplayer/GameInformationPanel.cs Outdated Show resolved Hide resolved

private void RenderMapPreview()
{
int maxPreviewHeight = 150;
Copy link
Member

Choose a reason for hiding this comment

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

This should be made a const

Comment on lines 70 to 73
leftColumnPositionX = 10;
rightColumnPositionX = Width / 2 - 10;
topStartingPositionY = 30;
mapPreviewPositionY = topStartingPositionY + (rowHeight * 2 + 15); // 2 Labels down, incase map name spills to next line
Copy link
Member

Choose a reason for hiding this comment

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

still think it should be easy to get rid of the magic numbers accross the code and make them constants, minor stuff like this accumulates and makes the code less readable

DXMainClient/DXGUI/Multiplayer/GameInformationPanel.cs Outdated Show resolved Hide resolved
@Metadorius Metadorius changed the title Show map preview and ping textures in game preview Show map preview in game preview Jan 5, 2025
@Metadorius Metadorius merged commit f13ba44 into develop Jan 5, 2025
3 checks passed
@SadPencil SadPencil deleted the feature/show-map-previews-on-game-listings branch January 5, 2025 23:46
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