-
-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spread Around New Tag Param Usage, Static Tag Registration, new Property format, and Command autoExecute #2355
Labels
Help Wanted
This is an issue we would prefer somebody contribute a PR for.
Improvement
Something could be better.
Comments
mcmonkey4eva
added
Improvement
Something could be better.
Help Wanted
This is an issue we would prefer somebody contribute a PR for.
labels
Aug 21, 2022
mcmonkey4eva
changed the title
Spread Around New Tag Param Usage and Static Tag Registration
Spread Around New Tag Param Usage, Static Tag Registration, and Command autoExecute
Aug 23, 2022
mcmonkey4eva
pushed a commit
that referenced
this issue
Aug 28, 2022
…sage. (#2364) * Update tag param usage for `ChunkTag` Contributes to issue #2355. * Update tag param usage for `AreaContainmentObject` Contributes to issue #2355. * Update tag param usage for `<ColorTag.mix[]>` Contributes to issue #2355. * Remove `null` checks in a few `AreaContainmentObject` tags. New tag processing makes sure the parameter is valid as pointed out by @tal5! * Remove `null` check in `<ColorTag.mix[]>`
Property FormatHere's the changes needed to update a property to modern format:
Part 2:
Note: some properties can not be easily updated, if eg they have inconsistent tag/mech names, or weird legacy subtag logic, or... |
mcmonkey4eva
changed the title
Spread Around New Tag Param Usage, Static Tag Registration, and Command autoExecute
Spread Around New Tag Param Usage, Static Tag Registration, new Property format, and Command autoExecute
Mar 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Help Wanted
This is an issue we would prefer somebody contribute a PR for.
Improvement
Something could be better.
This is an open-ended issue intended as an easy goal for contributors to help Denizen and learn to make good PRs, without having to dive deep into complicated codework.
This comes in a few parts:
Tag Param Usage
There's a bunch of old tags that look like this:
The attribute processing is now automated, and so the tag registration should now look like this:
This exclusively applies to tags that have a MANDATORY parameter. Any tag that lacks the
if (!attribute.hasParam()) {
check prefix probably isn't mandatory. Check the tag meta, if the param has()
on it, it's not mandatory, so don't change it.Not all tags had
echoError
in them, they should still be updated to the new format.Any PR that updates a significant number of tags to use this new format would be appreciated.
Static Tag Registration
This one requires a little bit more care. The old tag registration method was
registerTag
, but there is now also aregisterStaticTag
for when a tag is static. A 'static' tag is one in which for any given exact input, the same exact output will always result.For example,
<DurationTag.sub[<#>]>
is static because the math is always the same.5s - 3s
is always2s
.On the other hand,
<util.random_boolean>
is not static as whether it returns 'true' or 'false' is different every time.Caution must be taken as many tags rely on external state factors.
<world[...]>
is non-static because worlds can be loaded or unloaded.LocationTag.material
is non-static because somebody might break or place a block there. Anything underEntityTag
is non-static as there's no such thing as a static Entity.Mechanism Registration
Mechanisms up until 2022 were made with a
matches
spam method... they are now registered. All the old mechs need to be updated to the new register method.(Note: sorta-exception for int/bool/etc. values as they need dedicated registration tools still)
Command autoExecute
Commands using the legacy
parseArgs
method or the interimscriptEntry.argAsX(...)
style argument handling should be updated to the newautoExecute
code-gen toolset.An example commit is here: DenizenScript/Denizen-Core@ce1ba87
Some commands have arguments that are not yet compatible with the new system and should be left as-is for now.
Care should be taken to ensure the all valid argument inputs remain valid, and that default values are properly handled. In some cases, the best default is
@ArgDefaultNull
with anif (arg == null) { arg = something(); }
check inside the method.Commands with multiple linear arguments of previously indeterminate order should check for and show the
outOfOrderArgs
deprecation warning as demonstrated here: DenizenScript/Denizen-Core@b361471#diff-19ffff3dc1082c07ac26398394cc14ae15b2fb6156edcbcdbbbefe643a02dbccR58Property Legacy Tags
Some properties (EntityTag and ItemTag properties) still use the ancient legacy
attribute.startsWith
style of tags. These need to be updated to modernregisterTag
style. Here's a (slightly outdated) example PR of this: https://github.com/DenizenScript/Denizen/pull/2334/filesEvent Determinations
Update old determination handlers to new
register
format.Depenizen Property Legacy Extensions
Depenizen extensions should just directly register rather than janking a property class, as seen by this upgrade example commit here: 246cb1c
Also Depenizen
Also for contributors looking for things to do, if you use Depenizen, consider updating the very old legacy Depenizen code -
startsWith
and other legacy formats are used throughout.Generally speaking, the priority order is:
The text was updated successfully, but these errors were encountered: