Skip to content

Add alternative methods for commandfilters#478

Closed
LivingInPortal wants to merge 1 commit intoalliedmodders:masterfrom
LivingInPortal:master
Closed

Add alternative methods for commandfilters#478
LivingInPortal wants to merge 1 commit intoalliedmodders:masterfrom
LivingInPortal:master

Conversation

@LivingInPortal
Copy link

I have a need to know who triggered the commandfilters so created this pull request.
This should not affect the origin API.

@KyleSanderson
Copy link
Member

I like the idea. However, your implementation for it seems really over engineered. I believe we can just pass the admin paramater...

Something like the following (untested) may work?

diff --git a/core/logic/smn_players.cpp b/core/logic/smn_players.cpp
index 1218959..b8f35ee 100644
--- a/core/logic/smn_players.cpp
+++ b/core/logic/smn_players.cpp
@@ -141,6 +141,7 @@ public: //ICommandTargetProcessor

                smtf->fun->PushString(info->pattern);
                smtf->fun->PushCell(ahc.getClone());
+               smtf->fun->PushCell(info->admin);
                cell_t result = 0;
                if (smtf->fun->Execute(&result) != SP_ERROR_NONE || !result)
                    return false;
diff --git a/plugins/include/commandfilters.inc b/plugins/include/commandfilters.inc
index 32e3d9c..be0e2b7 100644
--- a/plugins/include/commandfilters.inc
+++ b/plugins/include/commandfilters.inc
@@ -138,9 +138,14 @@ stock ReplyToTargetError(client, reason)
  *
  * @param pattern       Pattern name.
  * @param clients       Array to fill with unique, valid client indexes.
+ * @param admin        Admin that called triggered the filter.
  * @return              True if pattern was recognized, false otherwise.
  */
-typedef MultiTargetFilter = function bool (const char[] pattern, Handle clients);
+funcenum MultiTargetFilter
+{
+   bool (const char[] pattern, Handle clients),
+   bool (const char[] pattern, Handle clients, int admin),
+}; 

 /**
  * Adds a multi-target filter function for ProcessTargetString().

@psychonic
Copy link
Member

The problem there is that you don't know if core is new enough or not to pass a valid value for that.

If you were to compile on SM with this, but at runtime have an older version, the param would have garbage. The requirement of the new native avoids that situation.

@KyleSanderson
Copy link
Member

True, that case can fall under FeatureType_Capability (which is not present in my snippet).

@psychonic psychonic added the WIP work in progress label Nov 4, 2017
@KyleSanderson
Copy link
Member

Closing this one (sorry @LivingInPortal - thank you very much for the contribution) in favour for the child PR. There's no guarantee it will be merged but it's the same functionality ultimately and it's what we both want out of this 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WIP work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants