Caraxi / SimpleTweaksPlugin

GNU Affero General Public License v3.0
162 stars 141 forks source link

[`CaseInsensitiveCommands`] Casexile, but a Tweak! #889

Closed KazWolfe closed 5 days ago

KazWolfe commented 1 week ago

Someone should probably verify this can be "adopted"... Anyways.

Command resolution order is: as-is command, case-insensitive game command, case-insensitive plugin command, default error response. Meaning:

I do not know how this plugin will interact with other plugins that perform their own command hooking; I suspect load order would kick in. So if, e.g., a plugin uses the same hook to listen for /sam for class-switch purposes, I'm not sure this one will necessarily resolve to that if the user types in /SAM.

Caraxi commented 1 week ago

I'll take a look at this in a couple weeks (maybe) once things are stable. Don't want a hotfix delayed because of a new tweak in the code lol

KazWolfe commented 1 week ago

Makes perfect sense. I'd like to give @RyouBakura some grace time to bring their plugin to API11 - or give consent for this migration as-is. Don't think this is anything critical, so it can hang around for a bit.

RyouBakura commented 1 week ago

Hey. I'm happy that you want to adopt this. Feel free to use any code that seems useful to you: https://gitlab.com/Bakura/casexile/-/blob/master/SlashCommandInterceptor.cs?ref_type=heads

Makes perfect sense. I'd like to give @RyouBakura some grace time to bring their plugin to API11 - or give consent for this migration as-is. Don't think this is anything critical, so it can hang around for a bit.

Casexile is broken since DT because the repo in which the XivCommon dependency is hosted doesn't accept PRs. And I didn't think it would be appropriate to ship Casexile with my own updated version of XivCommon because some poor maintainer would have to dig through all the bloat that comes with it to make sure I didn't add any malicious code lines.

YsEmei commented 1 week ago

Would it be possible to add Command Alias support too?

image

KazWolfe commented 1 week ago

Hey. I'm happy that you want to adopt this. Feel free to use any code that seems useful to you.

Neat, thank you for the confirmation. I did take a look at your repo but decided to rewrite from scratch to better handle certain edge cases.

Would it be possible to add Command Alias support too?

This is a question for Cara, but I suspect that it'd be better to add case insensitivity to that Tweak rather than perform a double-hijack here. I could probably add support to this tweak directly, but I'm unsure how that would affect the execution flow and if it'd further mess with context.

Caraxi commented 1 week ago

I added support for command alias

KazWolfe commented 1 week ago

Looks good and that should work, since you'd still be redirecting the alias prior to the command handler's initial invokes.