Firework effect section #5542

TheLimeGlass · 2023-03-21T10:15:43Z

Description

Pull request taking over from stale #4793

Turns the launch firework effect into an effect section.

Takes into consideration the bug fixes and enhances to the EffFireworkLaunch since.

Requesting same reviewers.

Co-authored-by: Ayham Al Ali <[email protected]>

Co-authored-by: APickledWalrus <[email protected]>

Co-authored-by: Ayham Al Ali <[email protected]>

Co-authored-by: APickledWalrus <[email protected]>

- Change power variable to Integer instead of Number so Number#intValue only has to be called once. - Changed Consumer type to Firework instead of ? extends Firework to remove Consumer cast. - Added parentheses in toString message - added NotNull annotation to getHandlers method - Removed SuppressWarnings from walk since it's no longer needed

Co-authored-by: Ayham Al Ali <[email protected]>

sovdeeth

Creating a local variable in the section does not carry over to the rest of the code.
The following should broadcast 2:

command test:
    trigger:
        launch ball large colored red, green and white fading to light green and black at (player's location, player's location) with duration 1:
            add 1 to {_test2}
            broadcast "a"
        broadcast "%{_test2}%"

[15:49:58 INFO]: Sahvde issued server command: /test
[15:49:58 INFO]: a
[15:49:58 INFO]: a
[15:49:58 INFO]: <none>

EffSecSpawn also doesn't handle this properly, but in a different way. See #6032

sovdeeth · 2024-04-07T13:42:35Z

Closing due to 6+ months of inactivity.

sovdeeth · 2024-05-04T13:22:27Z

src/main/java/ch/njol/skript/sections/EffSecFireworkLaunch.java

+	"\ttrigger:",
+		"\t\tlaunch a firework with effects ball large coloured red at player:",
+			"\t\t\tset metadata value \"cancel damage\" of event-entity to true",


Suggested change

"\ttrigger:",

"\t\tlaunch a firework with effects ball large coloured red at player:",

"\t\t\tset metadata value \"cancel damage\" of event-entity to true",

"\ttrigger:",

"\t\tlaunch a firework with effects ball large coloured red at player:",

"\t\t\tset metadata value \"cancel damage\" of event-entity to true",

APickledWalrus · 2024-06-06T19:37:52Z

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

@@ -157,7 +159,7 @@ public String toString(@Nullable Event event, boolean debug) {
 			default:
 				assert false;
 		}
-		return "the last " + word + " " + type;
+		return "last " + word + " " + type;


Suggested change

return "last " + word + " " + type;

return "the last " + word + " " + type;

it's better to be verbose for toString IMO

APickledWalrus · 2024-06-06T19:41:46Z

src/main/java/ch/njol/skript/sections/EffSecFireworkLaunch.java

+		if (sectionNode != null)
+			trigger = loadCode(sectionNode, "firework launch", FireworkSectionLaunchEvent.class);


It would be good to include the logic from EffSecSpawn that forbids delays.

Skript/src/main/java/ch/njol/skript/sections/EffSecSpawn.java

Lines 125 to 133 in 0355df1

if (sectionNode != null) {

AtomicBoolean delayed = new AtomicBoolean(false);

Runnable afterLoading = () -> delayed.set(!getParser().getHasDelayBefore().isFalse());

trigger = loadCode(sectionNode, "spawn", afterLoading, SpawnEvent.class);

if (delayed.get()) {

Skript.error("Delays can't be used within a Spawn Effect Section");

return false;

}

}

APickledWalrus · 2024-06-06T19:42:54Z

src/main/java/ch/njol/skript/sections/EffSecFireworkLaunch.java

+	public String toString(@Nullable Event event, boolean debug) {
+		return "launch fireworks " + effects.toString(event, debug) +
+				" at " + locations.toString(event, debug) +
+				" timed " + (power != null ? power.toString(event, debug) : "1");


Suggested change

" timed " + (power != null ? power.toString(event, debug) : "1");

" with duration " + (power != null ? power.toString(event, debug) : "1");

I think duration is a more familiar term for fireworks

APickledWalrus · 2024-06-06T19:43:21Z

src/main/java/ch/njol/skript/sections/EffSecFireworkLaunch.java

+			World world = location.getWorld();
+			if (world == null)
+				continue;
+			@SuppressWarnings("deprecation")


should likely be a //noinspection deprecation

sovdeeth · 2024-11-16T00:08:50Z

Closing due to inactivity

TFSMads and others added 26 commits June 9, 2022 10:52

Added Firework Launch Effect Section and removed Firework Launch effect.

be31180

Merge branch 'master' into feature/laucnhEffSec

e90f26e

Update src/main/java/ch/njol/skript/sections/EffSecFireworkLaunch.java

08c9164

Co-authored-by: Ayham Al Ali <[email protected]>

Update src/main/java/ch/njol/skript/sections/EffSecFireworkLaunch.java

2c89cba

Co-authored-by: Ayham Al Ali <[email protected]>

Update src/main/java/ch/njol/skript/sections/EffSecFireworkLaunch.java

ef7885b

Co-authored-by: APickledWalrus <[email protected]>

Update src/test/skript/tests/syntaxes/sections/EffSecFireworkLaunch.sk

50e0a26

Co-authored-by: APickledWalrus <[email protected]>

Update src/main/java/ch/njol/skript/sections/EffSecFireworkLaunch.java

d3bf799

Co-authored-by: APickledWalrus <[email protected]>

Update EffSecFireworkLaunch.java

33d8f6a

Update EffSecFireworkLaunch.java

99be168

Merge branch 'master' into feature/laucnhEffSec

ae10ac5

Merge branch 'master' into feature/laucnhEffSec

b13ad8c

Changed from %number% to %-number%

9b189d6

Update EffSecFireworkLaunch.java

6dc151c

Update src/main/java/ch/njol/skript/sections/EffSecFireworkLaunch.java

6235d08

Co-authored-by: Ayham Al Ali <[email protected]>

Update src/test/skript/tests/syntaxes/sections/EffSecFireworkLaunch.sk

6f0157d

Co-authored-by: Ayham Al Ali <[email protected]>

Merge branch 'master' into feature/laucnhEffSec

c46239a

Update src/main/java/ch/njol/skript/sections/EffSecFireworkLaunch.java

6157d51

Co-authored-by: APickledWalrus <[email protected]>

Update src/main/java/ch/njol/skript/sections/EffSecFireworkLaunch.java

456282a

Co-authored-by: APickledWalrus <[email protected]>

Update EffSecFireworkLaunch.java

8fc13c0

Check if varibles from section is carried over to the initial section

242717b

Merge branch 'master' into feature/laucnhEffSec

a4d5d81

Apply suggestions from code review

251c06e

Co-authored-by: Ayham Al Ali <[email protected]>

Take over stale pull request

31b8dc6

Apply changes to the new class

fc358a2

Final touches

28222cc

TheLimeGlass requested review from AyhamAl-Ali and APickledWalrus March 21, 2023 10:15

TheLimeGlass mentioned this pull request Mar 21, 2023

Added Firework Launch Effect Section and removed Firework Launch effect. #4793

Closed

Fix incorrect examples

28eef10

TheLimeGlass added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Mar 21, 2023

Moderocky approved these changes Sep 8, 2023

View reviewed changes

Moderocky force-pushed the master branch 2 times, most recently from bd134d0 to 3f08853 Compare September 16, 2023 16:59

Moderocky changed the base branch from master to dev/feature September 18, 2023 10:23

sovdeeth requested changes Sep 18, 2023

View reviewed changes

sovdeeth closed this Apr 7, 2024

TheLimeGlass reopened this Apr 29, 2024

TheLimeGlass and others added 2 commits April 29, 2024 15:24

Merge branch 'dev/feature' into feature/firework-effect-section

384be9f

Change collect order of local variables

9559c50

TheLimeGlass requested a review from sovdeeth April 29, 2024 21:33

TheLimeGlass added 2 commits April 29, 2024 15:50

Fix old bukkit consumer issues with spawning firework

2fa34d4

Change collect order of local variables

3ff79b4

sovdeeth approved these changes May 4, 2024

View reviewed changes

APickledWalrus requested changes Jun 6, 2024

View reviewed changes

Merge branch 'dev/feature' into feature/firework-effect-section

3277e4b

sovdeeth added 2.9 Targeting a 2.9.X version release and removed 2.9 Targeting a 2.9.X version release labels Jun 30, 2024

sovdeeth closed this Nov 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Firework effect section #5542

Firework effect section #5542

TheLimeGlass commented Mar 21, 2023 •

edited

Loading

sovdeeth left a comment •

edited

Loading

sovdeeth commented Apr 7, 2024

sovdeeth May 4, 2024

APickledWalrus Jun 6, 2024

APickledWalrus Jun 6, 2024

APickledWalrus Jun 6, 2024

APickledWalrus Jun 6, 2024

sovdeeth commented Nov 16, 2024

	return "last " + word + " " + type;
	return "the last " + word + " " + type;

		if (sectionNode != null)
		trigger = loadCode(sectionNode, "firework launch", FireworkSectionLaunchEvent.class);

	if (sectionNode != null) {
	AtomicBoolean delayed = new AtomicBoolean(false);
	Runnable afterLoading = () -> delayed.set(!getParser().getHasDelayBefore().isFalse());
	trigger = loadCode(sectionNode, "spawn", afterLoading, SpawnEvent.class);
	if (delayed.get()) {
	Skript.error("Delays can't be used within a Spawn Effect Section");
	return false;
	}
	}

	" timed " + (power != null ? power.toString(event, debug) : "1");
	" with duration " + (power != null ? power.toString(event, debug) : "1");

Firework effect section #5542

Firework effect section #5542

Conversation

TheLimeGlass commented Mar 21, 2023 • edited Loading

Description

sovdeeth left a comment • edited Loading

Choose a reason for hiding this comment

sovdeeth commented Apr 7, 2024

sovdeeth May 4, 2024

Choose a reason for hiding this comment

APickledWalrus Jun 6, 2024

Choose a reason for hiding this comment

APickledWalrus Jun 6, 2024

Choose a reason for hiding this comment

APickledWalrus Jun 6, 2024

Choose a reason for hiding this comment

APickledWalrus Jun 6, 2024

Choose a reason for hiding this comment

sovdeeth commented Nov 16, 2024

TheLimeGlass commented Mar 21, 2023 •

edited

Loading

sovdeeth left a comment •

edited

Loading