SkriptLang
Add SkriptAddon#asOrigin method
Suggestion
Add a SkriptAddon#asOrigin method into the skript addon class
Why?
The main reason for this is to provide addons a more obvious way of obtaining a SyntaxOrigin instead of going through the static method of SyntaxOrigin.of(SkriptAddon addon), this also allows addons to internally cache their origin and remove the creation of a new AddonOrigin each time someone wants to use it.
While they can cache it themselves it makes more sense for an addon instance to remember it themselves
Other
No response
Agreement
- [x] I have read the guidelines above and affirm I am following them with this suggestion.
I would prefer to keep this as is. I think using the static method in SyntaxOrigin makes more sense
I would prefer to keep this as is. I think using the static method in SyntaxOrigin makes more sense
Just out of well a need to understand, can you explain what makes more sense about it?
Syntax origin itself is intended for addons to say where the element used is from, in other words highly related to addons, the only exception to this is when we're using the old registration api in which skript has to call an internal method to find the origin, if it can find the addon instance it makes it, otherwise it uses a BukkitSyntaxOrigin as a default
The only other way SyntaxOrigin itself might be used is within skript-reflect to separate custom expressions from built in expressions.
I just don't see how having to call a static method in SyntaxOrigin is a problem. If there is legitimate reason to cache the result, I would support adding caching to the existing method but I don't think it needs to be solved with a new instance method on SkriptAddon.
I just don't see how having to call a static method in SyntaxOrigin is a problem. If there is legitimate reason to cache the result, I would support adding caching to the existing method but I don't think it needs to be solved with a new instance method on SkriptAddon.
I never said this was a problem, while it is a problem that there's nothing explaining to addons how to retrieve a SyntaxOrigin or what it even is, it was a QoL utility method so addons don't have to go looking around for how.
there is no real reason to cache it, however doing so at least removes the fact everytime SyntaxOrigin.of is called it's creating a new object instance
I guess I was assuming there is an issue with the way it is currently done in order to justify adding a new way. I'd rather just stick with (or add caching to) the existing solution. I'm also not sure if it is actually intended that each syntaxorigin instance is separate.
I'm also not sure if it is actually intended that each syntaxorigin instance is separate.
the way they work is retain a reference to a SkriptAddon instance, when it calls the internal method for getSyntaxOrigin it goes through the addons under skript's registry, there's nothing in it that requires unique instances if they only consist of an addon instance or nothing
Based on our discussion in the linked PR, we won't be adding this method. However, I am actively exploring other ways to improve the origin system.
I mean in truth origin doesn't really have a place, from my perspective this shouldn't be needed if/when syntax registries get per-addon storage.
There's no point for syntaxdata to hold this information besides maybe stacktraces but a quick look over them is all that's needed.