Spawners Incoming #7477

Burbulinis · 2025-01-19T18:06:24Z

Description

This PR aims to add advanced support for interacting with base spawners, spawners, trial spawner configurations and trial spawners.

PR #7009 was abandoned, thus this PR was made.

Everything here is only supported for versions newer than 1.21+, making this PR a breaking change because of ExprSpawnerType

TODO: tests, remove the enum class info for equipment slots after the merge of #7194

Target Minecraft Versions: 1.21+
Requirements: 1.21+
Related Issues: #6856, #5846

# Conflicts: # src/main/java/ch/njol/skript/Skript.java # src/main/java/ch/njol/skript/classes/data/SkriptClasses.java

TheAbsolutionism

This is just the first batch.
I would also like to note, imo, for all occurrences when inside of the get method of expressions. If the condition was unmet, instead of returning new Object[0] with Object being what the type needs to be, it should return null.

TESTS

TheAbsolutionism · 2025-01-19T18:40:50Z

src/main/java/ch/njol/skript/expressions/ExprWeight.java

+import org.bukkit.event.Event;
+import org.jetbrains.annotations.Nullable;
+
+public class ExprWeight extends SimplePropertyExpression<Object, Integer> {


src/main/java/ch/njol/skript/expressions/ExprWeight.java

TheAbsolutionism · 2025-01-19T18:43:38Z

src/main/java/ch/njol/skript/expressions/ExprWeight.java

+				continue;
+
+			if (!weighted.supportsWeightChange()) {
+				error("The weight of this object cannot be changed.");


Only problem I have with this, is that it doesn't say what object that was provided cannot be changed.

It provides the line and file, but sure that is fair

TheAbsolutionism · 2025-01-19T18:43:51Z

src/main/java/ch/njol/skript/expressions/ExprWeight.java

+
+	@Override
+	protected String getPropertyName() {
+		return "the weight";


Suggested change

return "the weight";

return "weight";

TheAbsolutionism · 2025-01-19T18:47:59Z

src/main/java/org/skriptlang/skript/bukkit/spawner/elements/conditions/CondIsActivated.java

+
+	@Override
+	protected String getPropertyName() {
+		return "spawner" + (isNegated() ? " is not " : " is ") + "activated";


Suggested change

return "spawner" + (isNegated() ? " is not " : " is ") + "activated";

return "an activated spawner";

The isNegated part is handled within PropertyCondition.

TheAbsolutionism · 2025-01-19T19:18:23Z

...g/skriptlang/skript/bukkit/spawner/elements/expressions/spawnrule/ExprSpawnRuleSkyLight.java

+		if (light > 15) {
+			warning("The sky light spawn level cannot be greater than 15, thus setting it to a value larger than 15 will do nothing.");
+			return;
+		} else if (light < 0) {
+			warning("The sky light spawn level cannot be less than 0, thus setting it to a value less than 0 will do nothing.");
+			return;
+		}


TheAbsolutionism · 2025-01-19T19:19:20Z

...a/org/skriptlang/skript/bukkit/spawner/elements/expressions/trialconfig/ExprTrialConfig.java

+
+	@Override
+	protected String getPropertyName() {
+		return type.name().toLowerCase() + " trial spawner config";


Suggested change

return type.name().toLowerCase() + " trial spawner config";

return type.name().toLowerCase(Locale.ENGLISH) + " trial spawner config";

TheAbsolutionism · 2025-01-19T19:20:42Z

...riptlang/skript/bukkit/spawner/elements/expressions/trialconfig/ExprTrialConfigMobCount.java

+
+	@Override
+	protected String getPropertyName() {
+		return (additional ? "additional " : "base ") + "spawns";


Needs to match property registered

TheAbsolutionism · 2025-01-19T19:21:30Z

...skript/bukkit/spawner/elements/expressions/trialconfig/ExprTrialConfigMobCountPerPlayer.java

+		if (additional) {
+			builder.append("additional");
+		} else {
+			builder.append("base");
+		}


Match patterns

TheAbsolutionism · 2025-01-19T19:23:07Z

.../skriptlang/skript/bukkit/spawner/elements/expressions/trialspawner/ExprTrackedEntities.java

+			if (players)
+				values.addAll(spawner.getTrackedPlayers());
+			else
+				values.addAll(spawner.getTrackedEntities());


Suggested change

if (players)

values.addAll(spawner.getTrackedPlayers());

else

values.addAll(spawner.getTrackedEntities());

if (players) {

values.addAll(spawner.getTrackedPlayers());

} else {

values.addAll(spawner.getTrackedEntities());

}

TheAbsolutionism · 2025-01-19T19:30:54Z

src/main/resources/lang/default.lang

@@ -2555,6 +2555,16 @@ input keys:
 	sneak: sneak key, sneaking key
 	sprint: sprint key, sprinting key

+//todo: remove after merge of equippable pr


Using a java comment💀

oh god i was brainwashed

Efnilite

epic start

src/main/java/org/skriptlang/skript/bukkit/spawner/SpawnerModule.java

src/main/java/org/skriptlang/skript/bukkit/spawner/elements/conditions/CondIsActivated.java

src/main/java/org/skriptlang/skript/bukkit/spawner/elements/conditions/CondIsTracking.java

src/main/java/org/skriptlang/skript/bukkit/spawner/util/SpawnerUtils.java

src/main/java/ch/njol/skript/lang/util/common/AnyWeight.java

ShaneBeee

Just a few things.
Pardon me if something of these have been mention already.
I was half way thru this when Effy submitted his!

ShaneBeee · 2025-01-19T19:49:59Z

src/main/java/org/skriptlang/skript/bukkit/spawner/SpawnerModule.java

+			return;
+
+		Classes.registerClass(new ClassInfo<>(TrialSpawnerConfig.class, "trialspawnerconfig")
+			.user("(trial ?)?spawner ?config(urations?)?")


Why is "trial" optional? To prevent possible conflicts in the future and/or with other addons, this should be required.

ShaneBeee · 2025-01-19T19:53:57Z

src/main/java/org/skriptlang/skript/bukkit/spawner/SpawnerModule.java

+		);
+
+		Classes.registerClass(new ClassInfo<>(SpawnerEntry.class, "spawnerentry")
+			.user("spawner ?entry")


this should probably be entr(y|ies) maybe?

ShaneBeee · 2025-01-19T20:04:47Z

src/main/java/org/skriptlang/skript/bukkit/spawner/elements/effects/EffSpawnedItem.java

+@Examples("set the spawned item of target block to diamond")
+@Since("INSERT VERSION")
+@RequiredPlugins("MC 1.21+")
+public class EffSpawnedItem extends Effect {


is there a reason this is an effect and not an expression?

there is no way to get the spawned item, only setting it is possible afaik

2 additional things

This is Paper exclusive

The setSpawnedItem just makes it the entity type be a dropped item, with the item being what was provided in setSpaawnedItem

ShaneBeee · 2025-01-19T20:06:24Z

src/main/java/org/skriptlang/skript/bukkit/spawner/elements/effects/EffTrialSpawnerTrack.java

+			.supplier(EffTrialSpawnerTrack::new)
+			.priority(SyntaxInfo.COMBINED)
+			.addPatterns(
+				"make [the] %blocks/trialspawnerconfigs% (1:start|stop) tracking %entities%",


I'm cofused, why two patterns? a Player is an entity, in the execute you could check if the entity is an instance of a player

if i am correct, tracking the player with the start tracking entity method works differently than player. I am not exactly sure how it works but I'll clean up the docs

ShaneBeee · 2025-01-19T20:12:15Z

...a/org/skriptlang/skript/bukkit/spawner/elements/expressions/basespawner/ExprSpawnerType.java

+	"",
+	"Base spawners are trial spawner configurations, spawner minecarts and creature spawners."
+})
+@Examples({


Should include some examples of your changers

ShaneBeee · 2025-01-19T20:32:19Z

...ptlang/skript/bukkit/spawner/elements/expressions/trialconfig/ExprTrialConfigLootTables.java

+
+	@Override
+	public String toString(@Nullable Event event, boolean debug) {
+		return "possible trial spawner rewards of" + getExpr().toString(event, debug);


this doesnt seem to be remotely related to the pattern?!?!

Alot of things were changed during the making of this and I completely forgot about the toString methods.

ShaneBeee · 2025-01-19T20:36:54Z

.../skriptlang/skript/bukkit/spawner/elements/expressions/trialspawner/ExprTrackedEntities.java

+			.supplier(ExprTrackedEntities::new)
+			.priority(PropertyExpression.DEFAULT_PRIORITY)
+			.addPatterns(
+				"[the] tracked (1:player[s]|entit(y|ies)) (from|of) %blocks/trialspawnerconfigs%",


should include spawner here, or something to destinguish this.
Spigot/Paper have tracked player/entity stuff, so we shouldnt create possible conflicts

...main/java/org/skriptlang/skript/bukkit/spawner/elements/expressions/ExprSpawnerLocation.java

ShaneBeee · 2025-01-19T20:40:07Z

src/main/java/org/skriptlang/skript/bukkit/spawner/SpawnerModule.java

+					} else {
+						builder.append("normal ");
+					}
+					builder.append("trial spawner configuration of trial spawner at ")


this worries me how insanely long this is going to be.

ShaneBeee · 2025-01-19T20:48:03Z

After all my comments I wanted to add one final comment.

This is my opinion (.... again... opinion).
I personally dont think this should be in Skript.
This is so insanely convoluted for basically one block (a spawner [yes I get it, the variants])
The docs are going to be all over the place, and so hard to figure out how to do anything.

I personally (again, my opinion) feel that this is better suited for its own addon.

Burbulinis · 2025-01-20T13:39:58Z

After all my comments I wanted to add one final comment.

This is my opinion (.... again... opinion). I personally dont think this should be in Skript. This is so insanely convoluted for basically one block (a spawner [yes I get it, the variants]) The docs are going to be all over the place, and so hard to figure out how to do anything.

I personally (again, my opinion) feel that this is better suited for its own addon.

I guess I agree with this... Any other opinions?

Burbulinis added 30 commits December 29, 2024 14:31

init commit

518223d

more stuff

7fe97ee

Optimize imports

4a46169

Trial spawner changes

a0873c8

Clean up

8fce16c

Merge remote-tracking branch 'origin/dev/feature' into feature/spawner

f5cbfaa

Use new registration API

c3ff81f

Clean up warnings

96f0fa7

Add expression and clean up patterns

9e0e7cc

Clean up patterns

7a1a809

Fix bug with SpawnRuleWrapper

82a8b97

Remove old spawner type expression & clean up

035c55b

Fix expression

aab1c96

Clean up, add AnyWeight and more expressions

7cae29f

Optimize imports

55c97a2

Effects, conditions and general clean up

f798cca

General clean-up

947416c

Separate events

45be4ac

Optimize imports

3239e51

Clean up

3eef5df

Add missing effects and expressions

d0d23df

Indentation

852135a

Add a default expr to AnyWeight

6779927

Merge remote-tracking branch 'origin/dev/feature' into feature/spawner

f56df91

# Conflicts: # src/main/java/ch/njol/skript/Skript.java # src/main/java/ch/njol/skript/classes/data/SkriptClasses.java

Clean up spawn rule light level expressions and add runtime warnings

366e2c4

Fix unnecessary check

2582f60

Optimize imports

33496a7

Clean up and remove redundant checks in SpawnRuleWrapper

d56ff41

Make sure nothing passes the holy line

d6e193d

Utilize runtime warnings

ac136f6

Burbulinis and others added 17 commits January 4, 2025 16:04

fix

bd034bd

le javadocs

3687530

whoooops

7cabe46

the damn docs

49aaec2

Optimize imports

e01f70e

Begin tests

3ac935d

more test stuff

c6e7dff

clean up

789687f

delete test file

21d3433

fix potential npe

631c56f

oopsies

a699071

clean up ExprSpawnerType

aaaaacc

Merge branch 'dev/feature' into feature/spawner

ee74ca3

fix tests

6fdf55a

Merge remote-tracking branch 'fork/feature/spawner' into feature/spawner

d90dd25

the tests

a54d7f0

fix tests

36920d2

This was referenced Jan 19, 2025

Adding more Spawner Features. #7009

Closed

Is Activated Condition #7014

Closed

TheAbsolutionism suggested changes Jan 19, 2025

View reviewed changes

TheAbsolutionism reviewed Jan 19, 2025

View reviewed changes

Efnilite added feature Pull request adding a new feature. breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) labels Jan 19, 2025

Efnilite requested changes Jan 19, 2025

View reviewed changes

ShaneBeee suggested changes Jan 19, 2025

View reviewed changes

Changes

e7b273e

Burbulinis marked this pull request as draft January 22, 2025 18:47

Burbulinis closed this Jan 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spawners Incoming #7477

Spawners Incoming #7477

Burbulinis commented Jan 19, 2025 •

edited

Loading

TheAbsolutionism left a comment

TheAbsolutionism Jan 19, 2025

TheAbsolutionism Jan 19, 2025

Burbulinis Jan 19, 2025

TheAbsolutionism Jan 19, 2025

TheAbsolutionism Jan 19, 2025

TheAbsolutionism Jan 19, 2025

TheAbsolutionism Jan 19, 2025

TheAbsolutionism Jan 19, 2025

TheAbsolutionism Jan 19, 2025

TheAbsolutionism Jan 19, 2025

TheAbsolutionism Jan 19, 2025

Burbulinis Jan 19, 2025

Efnilite left a comment

ShaneBeee left a comment

ShaneBeee Jan 19, 2025

ShaneBeee Jan 19, 2025

ShaneBeee Jan 19, 2025

Burbulinis Jan 19, 2025

TheAbsolutionism Jan 19, 2025

ShaneBeee Jan 19, 2025

Burbulinis Jan 20, 2025

ShaneBeee Jan 19, 2025

ShaneBeee Jan 19, 2025

Burbulinis Jan 20, 2025

ShaneBeee Jan 19, 2025

ShaneBeee Jan 19, 2025

ShaneBeee commented Jan 19, 2025

Burbulinis commented Jan 20, 2025 •

edited

Loading

	return "spawner" + (isNegated() ? " is not " : " is ") + "activated";
	return "an activated spawner";

	return type.name().toLowerCase() + " trial spawner config";
	return type.name().toLowerCase(Locale.ENGLISH) + " trial spawner config";

Spawners Incoming #7477

Spawners Incoming #7477

Conversation

Burbulinis commented Jan 19, 2025 • edited Loading

Description

TheAbsolutionism left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Efnilite left a comment

Choose a reason for hiding this comment

ShaneBeee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShaneBeee commented Jan 19, 2025

Burbulinis commented Jan 20, 2025 • edited Loading

Burbulinis commented Jan 19, 2025 •

edited

Loading

Burbulinis commented Jan 20, 2025 •

edited

Loading