roflmuffin / CounterStrikeSharp

CounterStrikeSharp allows you to write server plugins in C# for Counter-Strike 2/Source2/CS2
https://docs.cssharp.dev
Other
741 stars 111 forks source link

added `Open` method to each menu type #385

Closed partiusfabaa closed 4 months ago

partiusfabaa commented 5 months ago
var menu = new ChatMenu("Test");
menu.AddMenuOption("test", (controller, option) => {});
menu.Open(player);
var menu = new ConsoleMenu("Test");
menu.AddMenuOption("test", (controller, option) => {});
menu.Open(player);
var menu = new CenterHtmlMenu("Test", this);
menu.AddMenuOption("test", (controller, option) => {});
menu.Open(player);
partiusfabaa commented 5 months ago

I think it's worth having the player value nullable for the Open function then fall back to opening the menu for every player returned by Utilities.GetPlayers(), what do you think?

I think it would be better to make a separate method ala OpenToAll and it will be more intuitive for people

roflmuffin commented 5 months ago

I don't think the menu instance itself should include any logic for opening menus for a given player, since this is handled by the menu manager. I think it would be better to implement these as extension methods for both CCSPlayerController and IMenu. The problem is that the HTML menu requires context to a BasePlugin while the other menus do not. In any case, I don't think the change to require a BasePlugin from the constructor of CenterHtmlMenu is a good change.

Iksix commented 5 months ago

It's a good idea.

Muinez commented 5 months ago

I don't think the menu instance itself should include any logic for opening menus for a given player, since this is handled by the menu manager. I think it would be better to implement these as extension methods for both CCSPlayerController and IMenu. The problem is that the HTML menu requires context to a BasePlugin while the other menus do not. In any case, I don't think the change to require a BasePlugin from the constructor of CenterHtmlMenu is a good change.

I think it's better than having a MenuManager because you can get confused and call the wrong method, having 1 single method Open simplifies the work because you don't have to think about which method to call, also the fact that HtmlMenu's BasePlugin is passed in the constructor and not at opening simplifies the work, let's say you can create a class that creates a menu with a specific Title and that returns an IMenu, with the current implementation to open a menu you have to know its type.