Skip to content

Rename TreeType ClassInfo code-name from structuretype to treetype#8636

Open
adraarda23 wants to merge 3 commits intoSkriptLang:dev/patchfrom
adraarda23:refactor/rename-structuretype-to-treetype
Open

Rename TreeType ClassInfo code-name from structuretype to treetype#8636
adraarda23 wants to merge 3 commits intoSkriptLang:dev/patchfrom
adraarda23:refactor/rename-structuretype-to-treetype

Conversation

@adraarda23
Copy link
Copy Markdown

Problem

The TreeType ClassInfo is registered under the code name "structuretype", which is misleading: trees are not structures, and the identifier blocks introducing an actual StructureType class in the future.

Solution

  • Rename the ClassInfo code name from "structuretype" to "treetype" in SkriptClasses.java.
  • Update the matching pattern placeholders: %structuretype(s)%%treetype(s)% in EffTree.java (2 sites) and EvtGrow.java (3 sites).
  • Update the localization key in default.lang: structuretype:treetype:.
  • Per @erenkarakal's suggestion on the issue, cleanup in the touched files: rename single-letter variables (s, o, t, b, e) to descriptive names, and drop unnecessary final modifiers on parameters, locals, and loop variables. Field finals (real immutability) are preserved.

The user-facing .user("tree ?types?", "trees?") patterns are not changed, so existing user scripts continue to parse the same way.

Testing Completed

  • ./gradlew build passes locally on a Java 25 toolchain (Paper 26.1.1 environment).
  • Verified via grep that no structuretype references remain in the codebase (Java, lang, resources).

I did not add new test scripts. EffTree and EvtGrow have no existing .sk or JUnit coverage, and I judged that adding it was outside the scope of a code-name rename — happy to add a Tree.sk regression test if reviewers prefer.

Supporting Information

  • Compatibility: the Java class StructureType is unchanged, so the EnumSerializer<>(StructureType.class) registration still produces the same serialized form — existing variable storage stays valid.
  • Breaking surface: the only consumer that could break is code referring to the code name string "structuretype" directly (e.g. third-party plugins doing class lookups by code name). The probability is very low; ShaneBeee already flagged this risk in the issue.

Completes: #8634
Related: none
AI assistance: Claude Code (Anthropic CLI, Opus 4.7) was used extensively. Claude searched the affected files, performed the renames and cleanup edits, drafted the commit message, and ran the local build. All scope decisions (whether to rename the Java class, which finals to remove, commit message wording) were made by me after reviewing Claude's proposals.

The code name "structuretype" was misleading — trees are not structures,
and the identifier blocked introducing an actual StructureType class.
Rename the ClassInfo code name, the matching pattern placeholders in
EffTree and EvtGrow, and the localization key in default.lang.

Also clean up the touched files: rename single-letter variables to
descriptive names and drop unnecessary `final` modifiers on parameters,
locals, and loop variables. Field finals are preserved.

Resolves SkriptLang#8634
@adraarda23 adraarda23 requested a review from a team as a code owner May 7, 2026 11:01
@adraarda23 adraarda23 requested review from cheeezburga and erenkarakal and removed request for a team May 7, 2026 11:02
Comment thread src/main/java/ch/njol/skript/util/StructureType.java Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not be named StructureType anymore, something like TreeVariant is better as TreeType is already taken by bukkit
make sure to update it in DefaultComparators too

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @erenkarakal went with TreeSpecies instead of TreeVariant for two
reasons from the existing codebase:

  1. The ClassInfo description already says "a tree species or a huge mushroom
    species" (SkriptClasses.java:380).
  2. Bukkit itself calls a TreeType value a species — StructureGrowEvent.getSpecies()
    returns one, and EvtGrow.java:175 already uses species as the local variable
    name when reading from that event.

It also fits the grouping semantics a bit better than "variant" (each value
holds a group of TreeTypes — e.g. REDWOOD = small + tall). Happy to
switch to TreeVariant if you'd prefer, just wanted to flag the reasoning.

Comment thread src/main/java/ch/njol/skript/classes/data/SkriptClasses.java Outdated
TreeType is taken by Bukkit, so use TreeSpecies — the term is already
used by the existing ClassInfo description ("a tree species or a huge
mushroom species") and by Bukkit's StructureGrowEvent.getSpecies(),
which returns the same kind of value this enum wraps.

Updates DefaultComparators, EffTree, EvtGrow, and ClassesTest to match.

Also drops the unnecessary "" + string coercion in TreeSpecies.fromName
and the toVariableNameString parser in SkriptClasses (Enum.name() never
returns null).
Copy link
Copy Markdown
Member

@erenkarakal erenkarakal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

almost there

Comment thread src/main/java/ch/njol/skript/events/EvtGrow.java Outdated
Comment thread src/main/java/ch/njol/skript/events/EvtGrow.java Outdated
Comment thread src/main/java/ch/njol/skript/effects/EffTree.java Outdated
EffTree: drop unnecessary `final` modifiers on parameters/locals/loop
variables and rename single-letter `e`, `l` to `event`, `location` —
the cleanup that was promised in the original commit description but
not actually applied to this file.

EvtGrow: switch `instanceof` casts in checkFrom/checkTo to pattern
variables for consistency.

DefaultComparators: fix the TreeSpecies-vs-TreeSpecies comparator,
which compared `s2` against itself and unconditionally returned
Relation.EQUAL. The `s1` parameter was unused.

Closes SkriptLang#8638
@adraarda23 adraarda23 requested a review from erenkarakal May 7, 2026 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants