Skip to content
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

Update visual effects #4123

Closed

Conversation

TPGamesNL
Copy link
Member

@TPGamesNL TPGamesNL commented Jun 27, 2021

Description

This change makes visual effects an expression instead of a literal, and it allows for multiple expressions in an effect's pattern. It also makes visual effects serializable again, which I broke in #3939.
It also adds the last two 1.17 particles, and adds a size parameter to the dust particle.

TODO: https://discord.com/channels/135877399391764480/672553438936301568/1013094898284646461 EffVisualEffect init entity check

TODO: fix CondIsBlock priority first, mb UnparsedLiteral stuff too #5809


Target Minecraft Versions: any
Requirements: none
Related Issues: #3939, #4070, #4196, #4381

@TPGamesNL TPGamesNL added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Jun 27, 2021
@TPGamesNL TPGamesNL force-pushed the fix/visual-effects-expressions branch from e5761b6 to 726b82f Compare June 27, 2021 19:38
@TPGamesNL
Copy link
Member Author

The reason the test fails, is because the condition air is a block is parsed as the CondCompare between air, the item type, and block, the class info.
Without this change, Skript would also try to parse it as CondCompare, but between air (item type) and shield block, the visual effect, because the pattern is (block [with a shield]|shield block), matching block. But this fails because Skript can't compare an item type with a visual effect, so the whole CondCompare is thrown out and parsing continues, which eventually gets to CondIsBlock, the right condition.
It can only parse this as a visual effect because visual effects are literals, but because this change makes visual effects expressions, this'll no longer happen, and it parses block as a class info, and Skript can compare an item type with a class info, so this is successful, and it is not parsed as CondIsBlock.

@TPGamesNL TPGamesNL force-pushed the fix/visual-effects-expressions branch from 733b4cb to 433be2a Compare July 12, 2022 15:55
Copy link
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

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

Nothing major

Should pretty much be fine as long as it's tested

Comment on lines 810 to +814
@Override
@Nullable
public VisualEffect parse(String s, ParseContext context) {
return VisualEffects.parse(s);
return null;
}
Copy link
Member

Choose a reason for hiding this comment

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

You can just get rid of the override for this method since canParse returns false.

import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.stream.Stream;

public class VisualEffects {

private static final boolean NEW_EFFECT_DATA = Skript.classExists("org.bukkit.block.data.BlockData");
private static final boolean HAS_REDSTONE_DATA = Skript.classExists("org.bukkit.Particle$DustOptions");
private static final boolean DUST_OPTIONS_EXISTS = true;
Copy link
Member

Choose a reason for hiding this comment

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

This can just be removed

@TPGamesNL TPGamesNL marked this pull request as draft July 20, 2022 12:36
TheLimeGlass and others added 12 commits June 5, 2023 23:37
add vector projection expression

Co-authored-by: LimeGlass <[email protected]>
…1-SNAPSHOT (SkriptLang#5739)

* Bump io.papermc.paper:paper-api

Bumps io.papermc.paper:paper-api from 1.19.4-R0.1-SNAPSHOT to 1.20-R0.1-SNAPSHOT.

---
updated-dependencies:
- dependency-name: io.papermc.paper:paper-api
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

* Update and rename paper-1.19.3.json to paper-1.20.json

* Update build.gradle

* Update gradle.properties

* Update default.lang

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: LimeGlass <[email protected]>
…1-SNAPSHOT (SkriptLang#5746)

* Bump io.papermc.paper:paper-api

Bumps io.papermc.paper:paper-api from 1.20-R0.1-SNAPSHOT to 1.20.1-R0.1-SNAPSHOT.

---
updated-dependencies:
- dependency-name: io.papermc.paper:paper-api
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

* Update build.gradle

* Update gradle.properties

* Update and rename paper-1.20.json to paper-1.20.1.json

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: LimeGlass <[email protected]>
* Add quit reason

* Cleanup

* Versioning
sovdeeth and others added 24 commits June 27, 2023 18:15
Replace Java 9 replace method with Skript's method
* Initial

* Support plural

* Merge with ExprChunk

* Fix

* Fix 2

* Cleanup class to obey code convention

* Requested changes

* Fix indent

* Indent docs and improved pattern

---------

Co-authored-by: LimeGlass <[email protected]>
@sovdeeth
Copy link
Member

Closing due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants