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

Improved menu closing #294

Merged
merged 6 commits into from
Feb 5, 2024
Merged

Improved menu closing #294

merged 6 commits into from
Feb 5, 2024

Conversation

B3none
Copy link
Collaborator

@B3none B3none commented Jan 28, 2024

Allow plugins to close the active menu in the MenuManager, also updated OnKeyPress to utilise the GetActiveMenu function and added the .Close() function to a base menu instance

…ed OnKeyPress to utilise the GetActiveMenu function and added the .Close() function to a base menu instance
@partiusfabaa
Copy link
Contributor

It might be easier to add a parameter, like var menu = new ChatMenu("title", true or false);, responsible for determining whether the menu will close after pressing the button. Alternatively, in the handler, you can return true or false. Depending on the returned value, the menu will be closed or remain open. I believe these two options are ideal.

@roflmuffin
Copy link
Owner

roflmuffin commented Jan 30, 2024

To fix the API compatibility issues you would need to: define the 3rd parameter not as optional, but as an overloaded method (since optional params are a compile time thing), i.e. instead of public static void OpenChatMenu(CCSPlayerController player, ChatMenu menu, bool closeOnSelect = true) you leave the old onepublic static void OpenChatMenu(CCSPlayerController player, ChatMenu menu), which just calls the new one public static void OpenChatMenu(CCSPlayerController player, ChatMenu menu, bool closeOnSelect) with true.

You also need to provide a default implementation for Close() that does nothing (at the interface level) which keeps the API compatibility in check I believe.

@B3none B3none marked this pull request as draft February 5, 2024 02:41
@B3none
Copy link
Collaborator Author

B3none commented Feb 5, 2024

Converted this back to a draft because I'm not happy with implementing the CloseOnSelect in the constructor and I believe it would be better as a menu setter. menu.CloseOnSelect = true; defaulting the behaviour to true (as it was previously implemented).

@B3none
Copy link
Collaborator Author

B3none commented Feb 5, 2024

I opted to use a PostSelectAction enum instead:

public enum PostSelectAction
{
    Close,
    Reset,
    Nothing
}

How it looks to people using a menu in a plugin:

menu.PostSelectAction = PostSelectAction.Close;

@B3none B3none marked this pull request as ready for review February 5, 2024 03:17
@B3none B3none merged commit 8fc926e into main Feb 5, 2024
4 checks passed
@B3none B3none deleted the fix/menu-closing branch February 5, 2024 12:26
@roflmuffin roflmuffin mentioned this pull request Feb 22, 2024
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.

None yet

3 participants