sourcemod icon indicating copy to clipboard operation
sourcemod copied to clipboard

AmbiguousMenu: Automagically create menu when multiple targets match

Open BotoX opened this issue 5 years ago • 2 comments

Added hack to make plugins open a menu with all possible targets on ReplyToTargetError COMMAND_TARGET_AMBIGUOUS.

Explanation: There are two clients in the server, one named gene, the other one "Ene ~special characters~". An admin issues "sm_slay Ene" and gets following error message: More than one client matched the given pattern. What this hack will do is: Use GetCmdArg(0, ...); to get the command name "sm_slay". Use GetCmdArgString(...); to get the arguments supplied to the command. Use GetLastProcessTargetString(...); (which was implemented in this commit) to retrieve the arguments that were passed to the last ProcessTargetString call. It will then pass this data to the DynamicTargeting plugin through its AmbiguousMenu native. The plugin will open up a menu on the client and list all targets which match the pattern that was supplied to ProcessTargetString. If the client selects a menu entry, FakeClientCommand will be used to re-execute the command with the correct target.

sourcemod.inc is kinda ugly

BotoX avatar Sep 02 '19 17:09 BotoX

The implementation is indeed hacky, it also requires a plugin recompile to make use of this feature. It also only works if plugins follow the following scheme:

if((target_count = ProcessTargetString(...)) <= 0) {
    ReplyToTargetError(client, target_count);
    return Plugin_Handled;
}

I haven't encountered a single plugin which does this different. Actually I believe it even was my intention to straight up only support plugins which do targeting the "correct way". Aka. if there is an error in ProcessTargetString then call ReplyToTargetError and exit. The error which is caught in this case is COMMAND_TARGET_AMBIGUOUS which would print More than one client matched the given pattern. So if this is the case (and the plugin author hasn't specifically passed dynamic=false to ReplyToTargetError) it'll check if SM has the needed capabilities: GetLastProcessTargetString and if the DynamicTargeting plugin is even loaded. https://github.com/alliedmodders/sourcemod/pull/1069/files#diff-04ed686e7e10bc10466c487fadc9ea8eR147 From there on everything should be smooth sailing and never cause any issues.

Then again I also can't guarantee that the plugin will return after calling ReplyToTargetError. If it calls ReplyToTargetError multiple times then this would create multiple ambiguous menus. If it calls ProcessTargetString and ReplyToTargetError again (maybe manipulating the args itself trying to be smart?) then this would also create multiple ambiguous menus. All in all mostly the plugins fault imo. and nothing really bad will happen because of it anyways.

How would you go about adding this in ProcessTargetString anyways? What if the plugin doesn't call ReplyToTargetError and exit but instead does some other crazy stuff instead.

I don't see the recompiling thing as a dealbreaker, at least that way if this would break a certain plugin it'd only do so if the author/user compiles it with the current SM codebase. It'd be easy for them to fix it and simply recompile. But I'm sure there's plenty of cases where people just download an .smx from somewhere and drop it into their server. Where they have no way of recompiling it if something is wrong. And if they want this feature in one of those old .smx then yeah they'll have to go the extra mile and recompile it. But we won't break their old .smx at least by merging this.

Looking over this again I still really like the way I've done this, minimal changes to SM and zero changes needed to plugin code. Almost the entire functionality is implemented in plugin code, only some glue has been added to SM core. Then again some people might want to have stuff like this as C++ code in the core, others don't mind or prefer that it's mostly plugin code.

BotoX avatar Jul 09 '20 12:07 BotoX

@asherkin what are your current thoughts for this one? I'm apathetic between either method as they're both improvements, but I'm optimistic that this will hopefully land and improve the targeting situation that's beneficial for all end-users.

KyleSanderson avatar Aug 16 '20 23:08 KyleSanderson