Rathena: Remove menu script command and implicit assignment of `@menu` variable by script engine

[secretdataz]

• rAthena Hash: 728a29d

• Client Date: Irrelevant

• Server Mode: Both

• Description of Issue:

When select or menu script command is used, the @menu temporary character variable is automatically set to index of player's choice on displayed menu.
This can lead to unintended behavior of other scripts if @menu is not carefully utilized (see #2557).

Another fact is that, select command is supposed to be a modernized replacement of menu command and has existed long enough for most modern scripts to utilize it. There is no reason except for backwards compatibility to keep the menu command.

Therefore, this issue is my proposal to remove this behavior of automatically assigning @menu and menu script command.

[anacondaq]

For around 12 years this very useful extension with temp char-based var was in the select command, thousands of scripts which using this var have been written, and now you came here and decide "Let's remove it because someone just did small mistake by incorrect usage of the feature".

Is information about select and @menu present at docs? Is it used in scripts? Yes. Actively. Is the feature help to write good scripts? Yes, for many dynamic menus, and different menus. How? By minimizing tons of stupid checks for vars where is required character based var which will be flushed when a player session will end, but while he is online, the var must be used for NPC. (i don't talk yet about scope vars like .@ and so on).

I'm too lazy to show examples, but if will be any drama, I can show real examples of code why it the @menu var is important.

And removing such useful things which exist for years, just because someone maybe was tired and did very small mistake -> not looks good at all.

Here is ~400 reasons: http://bfy.tw/Ev1c + extra 100 reasons http://bfy.tw/Ev3E, and extra 200 reasons: http://bfy.tw/Ev3H
And around 50-60 reasons already in the code. And I do not talk about hundreds of custom scripts
Yes, you can rewrite them by using switch/case, but for what? Are these scripts work? Why is the need to touch things which just works fine? For what to make extra work, while there is tons of other more interesting things to do? Like for example fixing bugs?)

And the last and most important thing for me: - why should I prove and convince you of anything like this basic and simple stuff? Is it really not clear that you do not need to touch things that work for years irreproachably? If you want to delete something - just argue this as best as possible, and do not kill the emulator and its compatibility. If you are so trying to increase the flow of orders by rewriting the NPC - then good luck to you. I'm not worried about myself, I can restore this function that you do planning remove, I'm worried about newbies, which will not be able to use other scripts and will have millions of bugs with old scripts. And the most problematic thing, that they will not be able even to understand what was happened because @menu will be always 0. If it's your plan and business model, it's ok, it will explain everything for me (real motivation), if not, then it looks strange, because of one small typo, decided an action to remove functionality, and not fixing the bug in the script.

Anyway, you can ignore my comment, and do as you wish, there is no problems or negative at all.
Cheers ❤️

Related to: https://github.com/rathena/rathena/issues/2557

[Atemo]

select return the index of the menu option picked, @menu isn't need for this command unlike menu command.
A lot of command should return scope variable .@ instead of @ $@ to store the informations.

• @menu should be removed from select
• @menu should be replaced by .@menu in menu command

However the skill usage when talking to NPCs (unofficial) is the main culsprit for https://github.com/rathena/rathena/issues/2557 for me

[Jeybla]

I like your ideas @Atemo and @secretdataz. But as @anacondaqq already mentioned: the downside for breaking the backward compatibility is too high in comparison to the benefit.

We should keep it and go for other ways like disabling skills or disable the ability to talk to other NPCs while being in callshops. I also see the real issue more in the script itself than in our script engine.

[lighta]

What if we somewhat let server admin decide ?
We activelyfor now end the support of @menu, and prevent his usage by some macro.
Uless you define that macro, in which case you are responsable if something like 2557 happen to you. (i.e like the #define _CRT_SECURE_NO_WARNINGS of windows).
Say #define MENU_INSECURE_NO_WARNINGS ?