Improve: allow isActive(...) and exists(...) to use an ID number
#6401
Conversation
With this PR passing an actual number as the first argument will use it as an ID number (provided it rounds to a zero or greater integer) and return a `1` or `0` if the corresponding Mudlet item is "active" or not (though like the unmodified one that returns a count of items of the selected type that are active it does not consider whether all the item's "ancestors" are active as well before including an item in the count). This can be used to identify the active status of temporary items (which are only identified by an ID number) - provided of course that they are around long enough to be checked. If the item does NOT exist then a `nil` and an error message will be returned instead. This ought to help with the sort of issue that Mudlet#6397 raises though a script using this revised function would, I think, be needed to do what is wanted there. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
|
Not sure if this addition is worthwhile really. Then again, would rather see a new function than overload existing API. |
|
OTOH It is impossible to determine the state of an existing temporary item. This is independent of whether the end-user generated it themselves and remembered/recorded the ID or not. Also, reusing/overloading the function name reduces the number of different functions that we have to document but extends the functionality in an, I think, logical/useful manner. |
Fair enough.
My point exactly. I would prefer to have many small concise functions which will do one thing exactly and well, rather than very few very complex functions which can do a dozen different things, if you know how to wield them for that case. While overloading is possible in c++ (but not c itself) it is rather uncommon in Lua, and I have pointed out some problems it may cause down the line already. |
This PR is an alternative/replacement for Mudlet#6401 that provides the functionality with a *new* function rather than overloading the existing `isActive(name as string, type as string)`. This is at the request of @Kebap who expressed a strong desire for this alternative approach to establishing the status of a Mudlet item by ID number rather than by name. As temporary items do not *have* names but they do possess a unique ID this will also work for them whereas the unmodified `isActive(...)` function can not. Unlike the `isActive(...)` function there can only be a single item that matches the ID so the return value is a boolean value rather than an integer count (of matching items that are active). Whilst developing the `isActiveID(...)` function I spotted that there was potentially a need for the related `existsID(...)` in the same way that there is an `exists(name, type)` alongside the `active(name, type)` function. Like the `isActiveID(...)`function compared to the `isActive(...)` the `existsID(...)` too only relates to a single item so also returns a boolean result. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
This PR is an alternative/replacement for Mudlet#6401 that provides the functionality with a *new* function rather than overloading the existing `isActive(name as string, type as string)`. This is at the request of @Kebap who expressed a strong desire for this alternative approach to establishing the status of a Mudlet item by ID number rather than by name. As temporary items do not *have* names but they do possess a unique ID this will also work for them whereas the unmodified `isActive(...)` function can not. Unlike the `isActive(...)` function there can only be a single item that matches the ID so the return value is a boolean value rather than an integer count (of matching items that are active). Whilst developing the `isActiveID(...)` function I spotted that there was potentially a need for the related `existsID(...)` in the same way that there is an `exists(name, type)` alongside the `active(name, type)` function. Like the `isActiveID(...)`function compared to the `isActive(...)` the `existsID(...)` too only relates to a single item so also returns a boolean result. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
Closing in favour of #6403... |
|
Reopened as this approach was deemed more acceptable in this case than #6403... |
|
Need to revise the |
Kebap
added
the
needs documentation
|
Can't review this code is above my level but some of my comments from the other proposal may hold true still. |
This has been selected to be the more desirable way of doing things compared to the alternative of Mudlet#6403. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
SlySven
changed the title
isActive(...) and exists(...) to use an ID number
|
📚 Documentation in Main wiki updated to (correctly) refer to handling ID numbers - the prior text incorrectly suggested that ID numbers already worked. See exists(...) and isActive(...).
|
| // The "isOptional" parameter is optional but modifies the error message to say | ||
| // that an argument is optional and it will default to not-optional parameters! | ||
| // HOWEVER it does not actually handle the absence of an argument that is | ||
| // supposed to BE optional - that has to be done by the caller before it | ||
| // makes the call... 8-P |
There was a problem hiding this comment.
This is a bit of a bummer but I think the logic here might confuse others, especially not native English speakers
There was a problem hiding this comment.
These are internal use functions though - only seen by us developers - who will need to remember this detail! 👿
There was a problem hiding this comment.
We have non-native English speaking developers (and would love more), can you reword this to be simpler?
There was a problem hiding this comment.
-// The "isOptional" parameter is optional but modifies the error message to say
-// that an argument is optional and it will default to not-optional parameters!
-// HOWEVER it does not actually handle the absence of an argument that is
-// supposed to BE optional - that has to be done by the caller before it
-// makes the call... 8-P
+// The (optional here) "isOptional" parameter only modifies the error message
+// given to the user to say that the parameter need not be provided. It does not
+// (until someone works out how to!) allow this function to deal with the argument
+// at the given pos not being present - so it is up to the function that calls this to
+// check for that BEFORE trying to call this function. This applies to all the
+// "getVerifiedXxxx" functions.| if (lua_type(L, pos) == LUA_TNUMBER) { | ||
| // use lua_tonumber(...) and round because lua_tointeger(...) can return | ||
| // oversized values (long long int?) on Windows which do not always fit | ||
| // into an int: |
SlySven
removed
the
needs documentation
Brief overview of PR changes/additions
With this PR passing an actual number as the first argument will use it as an ID number (provided it rounds to a zero or greater integer) and return a
1or0if the corresponding Mudlet item is "active" or not / exists or not (though like the unmodified one that returns a count of items of the selected type that are active it does not consider whether all the item's "ancestors" are active as well before including an item in the count).This can be used to identify the active status of temporary items (which are only identified by an ID number) - provided of course that they are around long enough to be checked. If the item does NOT exist then a
niland an error message will be returned instead.N. B.
isActive(...)will return0if the item (when given an ID number) or items (when given a name) does NOT exist - this might not be what is expected but is how the previous "name-only" form worked. OTOH ID numbers less than zero are never (:crossed_fingers:) going to occur and those will provoke anil+ error message if used.Motivation for adding to Mudlet
This came about as a result of a comment in the Discord #help channel:
Other info (issues closed, discussion etc)
This ought to help with the sort of issue that the OP raised (#6397) though a script using this revised function would, I think, be needed to do what is wanted there.
Signed-off-by: Stephen Lyons slysven@virginmedia.com