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

Modify GameInformationPanel to optionally display as an invite #633

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

11EJDE11
Copy link
Contributor

@11EJDE11 11EJDE11 commented Jan 9, 2025

Currently when you receive an invite to join a game, you can only get information about the game host and game name. This PR modifies game invite requests to show as a GameInformationPanel instead of ChoiceNotificationBox so you get the nice map thumbnail and all the other information that is normally shown about a game.

It adds Accept/Decline buttons to the GameInformationPanel which are visible if [panel].IsInvite is set.

image

First timer here so apologies if I am going about this incorrectly.

Update invite handler to use GameInformationPanel instance instead of ChoiceNotificationBox
@Saint146
Copy link
Contributor

Saint146 commented Jan 9, 2025

I'm also a first timer but damn this looks nice. Maybe place the buttons farther from each other or all the way in the left and right corners?

@Metadorius
Copy link
Member

Metadorius commented Jan 9, 2025

Transmitting this on behalf of @Rampastring who doesn't have convenient access to GitHub at the moment:

Could be done better by creating a game invitation panel/window that has the game information panel as a child

Copy link

github-actions bot commented Jan 9, 2025

Nightly build for this pull request:

Add GameInvitePanel window
Add check to ensure we are not accepting an invite to a lobby that we are already a part of
@11EJDE11
Copy link
Contributor Author

11EJDE11 commented Jan 9, 2025

Thanks for the feedback Saint, Meta, and Rampa. A game invitation panel makes much more sense. I have made the requested changes.

A bug has also been fixed where if you accept an invite to a game lobby that you are already a part of, it will no longer leave lobby and attempt to join again. Previously this would quite often fail.

There is a slight change to the layout from my last commit:
image

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.

Looking good so far, I have a couple of questions though.

  1. Have you checked whether you can customize it through the INI?
  2. Not sure what special logic you have for alpha?
  3. Is the panel updated on game changes if the user doesn't accept the invite for long enough?
    • Not necessarily in scope of this PR, but what happens if the game gets abandoned and the window is still open?

I also suggest to perhaps remove borders from the inner panel somehow and re-add information on who invited you.

DXMainClient/DXGUI/Multiplayer/GameInvitePanel.cs Outdated Show resolved Hide resolved
DXMainClient/DXGUI/Multiplayer/GameInvitePanel.cs Outdated Show resolved Hide resolved
DXMainClient/DXGUI/Multiplayer/GameInvitePanel.cs Outdated Show resolved Hide resolved
DXMainClient/DXGUI/Multiplayer/GameInvitePanel.cs Outdated Show resolved Hide resolved
DXMainClient/DXGUI/Multiplayer/GameInvitePanel.cs Outdated Show resolved Hide resolved
@Metadorius Metadorius self-assigned this Jan 10, 2025
11EJDE11 and others added 3 commits January 11, 2025 02:43
Co-authored-by: Kerbiter <crabiter@vivaldi.net>
Co-authored-by: Kerbiter <crabiter@vivaldi.net>
Co-authored-by: Kerbiter <crabiter@vivaldi.net>
@pzhlkj6612
Copy link
Contributor

Hey, @11EJDE11! A pro tip about GitHub:

To quickly incorporate more than one suggested change into a single commit, you can also apply suggested changes as a batch. ...

from: Reviewing proposed changes in a pull request - GitHub Docs

Use UIDesignConstants for button sizes
Remove border being drawn on GameInformationPanel
Remove unneeded alpha code
Other minor fixups
@11EJDE11
Copy link
Contributor Author

Thanks for the review, Metadorius.

  1. Regarding the INIs - not too sure if they can be customised through there. I'm not calling ReadINIForControl so I assume not? I don't believe it was doing anything with the INIs before though either.

  2. Alpha stuff was a leftover. I've removed it now.

3.1) I have made a change so the panel will update when the game information updates.

3.2) The invite will disappear if the game has closed or the host is offline.

I've removed the border from the inner panel and have added the invitee's name back in (note: invites can only be from hosts and the host name is already on the inner panel).

Also I am new to Github and all this jazz - do I click Resolve Conversations on the ones above? I have switched to nameof as you suggested, and I am now using the button constants for size. There is no constant I could see for the padding amount so I have copied the amount from another area that had padding.

And thanks pzhlkj6612 - I could have sworn that was greyed out! Please keep the tips coming; I need them. Pretty sure I've done the last commit a bit weird. Not sure why there is a second one for changing the map to readonly. When I pushed it said I was behind so had to update to the latest branch. Did that and there was a conflict, chose the newer/local copy and then it put that one line into another commit. Pushed that and here we are.

New panel:
image

@Metadorius
Copy link
Member

@11EJDE11 You're welcome!

Regarding the INIs - not too sure if they can be customised through there. I'm not calling ReadINIForControl so I assume not? I don't believe it was doing anything with the INIs before though either.

This should be done. Generally the point is to have all of the client customizable since there are many users (as in modders and game maintainers) who use it and they all have different customization needs. , For example, look at GameCreationWindow -- it is customizable via INI.

I am not sure if I am a fan of combining title that is all in caps and the username.

do I click Resolve Conversations on the ones above?

Yeah.

There is no constant I could see for the padding amount so I have copied the amount from another area that had padding.

I propose to introduce one. It is used in GameInformationPanel as well.

I could have sworn that was greyed out!

That's the GitHub unintuitivity for you. It is only available in the files tab.

@pzhlkj6612
Copy link
Contributor

Regarding the INIs ...

... who use it and they all have different customization needs. ...

Another example is, I'd like to hide the "GAME INFORMATION" line in the dialog. I can achieve it easily if you provide INI-ish way to customize it.

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.

4 participants