Rework entity visibility #6867

Efnilite · 2024-07-05T11:45:01Z

Description

Adds support for the hideEntity, showEntity and canSee(Entity) and removes the player-only equivalents, which are functionally equivalent. Also updates the Hidden Players syntax to match the current style guide and to improve the naming of the variable required for reveal hidden players of {_p}.

I wasn't sure on how to do the test for the can see condition, as this will have to be done with JUnit to mock the player, but I couldn't get it to work properly with canSee. pls help :)

Target Minecraft Versions: 1.19+ (Skript 2.10)
Requirements: none
Related Issues: #6795

cheeezburga · 2024-07-05T13:28:41Z

Can this just be put into the already existing CanSee condition and PlayerVisibility effect instead of making brand new ones?

Efnilite · 2024-07-05T14:44:35Z

Can this just be put into the already existing CanSee condition and PlayerVisibility effect instead of making brand new ones?

Not sure about that, since the entity one has only been around since Spigot 1.19, but idk

cheeezburga · 2024-07-05T15:06:37Z

Can this just be put into the already existing CanSee condition and PlayerVisibility effect instead of making brand new ones?

Not sure about that, since the entity one has only been around since Spigot 1.19, but idk

You should just be able to change which kind of type is accepted in the pattern based on whether or not a certain method exists, if that makes sense.

sovdeeth · 2024-07-05T15:15:40Z

You can just change it entirely since 2.10 will be 1.19+

ShaneBeee

Just a few little things

src/main/java/ch/njol/skript/effects/EffEntityVisibility.java

src/main/java/ch/njol/skript/conditions/CondCanSeeEntity.java

cheeezburga

Just some small things I saw - I think some of them are just preference so up to you. Stuff like the star imports tho are pretty up there.

src/main/java/ch/njol/skript/conditions/CondCanSee.java

src/main/java/ch/njol/skript/effects/EffEntityVisibility.java

src/main/java/ch/njol/skript/conditions/CondCanSee.java

src/main/java/ch/njol/skript/effects/EffEntityVisibility.java

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

src/main/java/ch/njol/skript/conditions/CondCanSee.java

src/main/java/ch/njol/skript/effects/EffEntityVisibility.java

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

sovdeeth

re: mock tests, you would need to provide your own logic implementation for canSee, since mocked objects have no inherent logic. I'm not sure if a junit test here is worth it.

src/main/java/ch/njol/skript/conditions/CondCanSee.java

src/main/java/ch/njol/skript/effects/EffEntityVisibility.java

Shouldn't be in the scope of this PR.

sovdeeth

what's the status of the junit tests?

Efnilite · 2024-07-14T11:05:20Z

what's the status of the junit tests?

i think the EffEntityVisibility test is fine, but not really sure how to improve the one for CondCanSee. maybe adding an entity to a collection when it's hidden and then canSee checking that list? suggestions welcome :)

sovdeeth · 2024-07-18T05:24:18Z

what's the status of the junit tests?

i think the EffEntityVisibility test is fine, but not really sure how to improve the one for CondCanSee. maybe adding an entity to a collection when it's hidden and then canSee checking that list? suggestions welcome :)

That'd work! As long as there's some basic logic implementation.

src/main/java/ch/njol/skript/effects/EffEntityVisibility.java

src/main/java/ch/njol/skript/conditions/CondCanSee.java

Co-authored-by: cheeezburga <47320303+cheeezburga@users.noreply.github.com>

Co-authored-by: Moderocky <admin@moderocky.com>

TheLimeGlass

Very good, very impressive.

src/main/java/ch/njol/skript/conditions/CondCanSee.java

src/test/java/org/skriptlang/skript/test/tests/syntaxes/conditions/CondCanSeeTest.java

src/test/java/org/skriptlang/skript/test/tests/syntaxes/effects/EffEntityVisibilityTest.java

…s/EffEntityVisibilityTest.java Co-authored-by: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com>

…ions/CondCanSeeTest.java Co-authored-by: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com>

* Fix gradle temp * Start on displays * Save * More work * update gradle.build * stash * Progress so far * Finish bulk add * Finalize so far * Minimize adventure api * Fix static class check for not 1.19.4 * Fix missing display classinfo in pattern under 1.19.4 * Fix missing quaternions classinfo in pattern under 1.19.4 * Make sure test runs on only 1.19.4 * Address reviews and fix java 8 * Apply suggestions from code review Co-authored-by: sovdee <ethansovde@gmail.com> * Move classes * Apply changes * Move classes * Class rename * Update to Spigot's fix to TextAlignment * Return null when using w of vector * Update src/main/java/ch/njol/skript/expressions/ExprXYZComponent.java Co-authored-by: sovdee <ethansovde@gmail.com> * Use brackets * Add entity hiding * Add property init values for displays * 1.19.4 check * Add rotate syntaxes * Change example * Fix tests * shadow syntax missing * Change setting * Revert change * Update src/main/java/ch/njol/skript/conditions/CondCanSee.java * Fix imports from master merge * Get tests working * Add color reset * Update ExprXYZComponent.java * Update DisplayData.java * Update DisplayData.java * Change the package naming and merge item into its own class * Add versioning for hide effect * Add versioning for hide effect * Change tests * Update submodule skript-aliases * Fix tests * Fix tests * Fix tests * Apply suggestions from code review Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com> * Apply changes * Add axis angle * Add axis angle * Update skript-aliases * fix merge error * Remove axisangle and only present quaternion to users * Add tests for basic quaternion features, cleanup and fix bugs with ExprXYZComponent, add tests Notable changes include changing float comparison behavior (needs discussion) and allowing multiple vectors/quaternions to be changed at once (will overwrite list indices, needs discussion) * Add ExprQuaternionAxisAngle and tests. Change float comparison to use dynamic epsilon value. * Temporary change to build.gradle for java 17 * Revert "Update src/main/java/ch/njol/skript/conditions/CondCanSee.java" This reverts commit 3da7b37. * Reverts entity hiding commits in favor of #6867 Shouldn't be in the scope of this PR. * version check for EQAA and displaydata cleanup * force 'rotation' in EQAA, fix classdef error, add itemtypes to block display entitydata pattern, add tests for display entitydata Not sure wh * Clean up colorOf, add tests, add alpha parameter to rgb function and color tostring, add rgba factory method + constructor Discussion needed surrounding type checking in colorof acceptChange * update Nullable annotations to jetbrains * Fix <1.19 errors (just for actions) and fix subtle issues with xyz component changer * rename package to bukkit * finish package refactor * Add tests for billboarding, rework display brightness and add tests, move joml utilities * tests + allow setting override to a single number * tests, cleanup, and bug fixes for glow override and hegiht/width * Interpolation and Shadow tests + correct reset value for shadow strength + guard against NaN and init * Tests and cleanup for teleportation duration, remove number from changers for tp duration and interpolation delay/duration * Transformation tests, cleanup, bugfixes. Removed unnecessary Joml utils class. * wrong version check for teleportation duration * Display view range tests, cleanup, finite guard, switch to Timespan#getAs * strip licenses * cleanup, tests for ItemDisplayTransfrom. Fixed default from being FIXED to NONE, changed lang for NONE to be no transform to avoid confusion * Remove ItemOfItemDisplay (redundant), cleanup/test the text display expressions. * small cleanup to textdisplayexprs and rename ItemFrameSlot to be more generic * rework ExprRotate * Completely rework rotation with new Rotator API. Add support for quaternions + displays in EffRotate * cleanup rotations - java 17 tests will fail until legacy tests are removed * Fix bugs with rotation, adds tests * Add drop shadow syntax and small cleanup of ExprTextOf * See through blocks syntax * Update ExprRotate.java * remove extra [a]'s * whoops! reverts a change to number accuracy config done for testing * Module-ize, package infos, remove mc version checks * rename display.expressions package, use for loops, move displaydata * display data improvements * requested changes 1 * update switches and remove banana * requested changes 2 * Apply suggestions from code review Co-authored-by: cheeezburga <47320303+cheeezburga@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: cheeezburga <47320303+cheeezburga@users.noreply.github.com> * fix expritemofentity description * Apply suggestions from code review Co-authored-by: cheeezburga <47320303+cheeezburga@users.noreply.github.com> * Update ExprDisplayHeightWidth.java * Apply suggestions from code review Co-authored-by: Patrick Miller <apickledwalrus@gmail.com> * requested changes 2 + rotator docs * requested changes * Apply suggestions from code review Co-authored-by: Patrick Miller <apickledwalrus@gmail.com> * requested changes! --------- Co-authored-by: sovdee <ethansovde@gmail.com> Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com> Co-authored-by: Moderocky <admin@moderocky.com> Co-authored-by: cheeezburga <47320303+cheeezburga@users.noreply.github.com> Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>

sovdeeth · 2024-10-13T03:44:56Z

@Efnilite conflicts :(

# Conflicts: # src/main/java/ch/njol/skript/conditions/CondCanSee.java # src/main/java/ch/njol/skript/effects/EffPlayerVisibility.java # src/main/java/ch/njol/skript/expressions/ExprHiddenPlayers.java

Efnilite · 2024-10-13T14:58:53Z

@sovdeeth should be good now

ShaneBeee suggested changes Jul 5, 2024

View reviewed changes

Efnilite changed the title ~~Add entity visibility~~ Rework entity visibility Jul 5, 2024

Efnilite requested a review from ShaneBeee July 5, 2024 22:18

cheeezburga reviewed Jul 5, 2024

View reviewed changes

sovdeeth requested changes Jul 7, 2024

View reviewed changes

Efnilite requested a review from sovdeeth July 7, 2024 16:59

sovdeeth requested changes Jul 12, 2024

View reviewed changes

src/main/java/ch/njol/skript/conditions/CondCanSee.java Show resolved Hide resolved

src/main/java/ch/njol/skript/effects/EffEntityVisibility.java Show resolved Hide resolved

src/main/java/ch/njol/skript/effects/EffEntityVisibility.java Outdated Show resolved Hide resolved

sovdeeth added a commit that referenced this pull request Jul 13, 2024

Reverts entity hiding commits in favor of #6867

a02074f

Shouldn't be in the scope of this PR.

Efnilite requested a review from sovdeeth July 13, 2024 20:32

sovdeeth approved these changes Jul 13, 2024

View reviewed changes

sovdeeth added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Jul 14, 2024

Efnilite requested a review from sovdeeth July 19, 2024 21:49

sovdeeth requested changes Jul 20, 2024

View reviewed changes

src/main/java/ch/njol/skript/effects/EffEntityVisibility.java Outdated Show resolved Hide resolved

src/main/java/ch/njol/skript/effects/EffEntityVisibility.java Outdated Show resolved Hide resolved

Efnilite requested a review from sovdeeth July 21, 2024 18:32

sovdeeth approved these changes Jul 22, 2024

View reviewed changes

Moderocky reviewed Aug 17, 2024

View reviewed changes

src/main/java/ch/njol/skript/conditions/CondCanSee.java Outdated Show resolved Hide resolved

Efnilite requested a review from Moderocky August 17, 2024 20:57

Moderocky approved these changes Aug 18, 2024

View reviewed changes

Efnilite added 6 commits August 18, 2024 16:50

init commit

99d8dad

fixes

a062c45

minor fixes

ef1f1b9

oops

872d4e6

fixed formatting

f246a10

replace player hiding with entity hiding

46204b0

Efnilite and others added 10 commits August 18, 2024 16:50

Apply suggestions from code review

f83885b

Co-authored-by: cheeezburga <47320303+cheeezburga@users.noreply.github.com>

fixed versions

3dee858

fixed ParseResult import

2e48789

added can see test

8f11231

fixed review stuff

8859cd6

updated var, param names

0b5a765

updated CondCanSeeTest

7a7f3e0

fixed space

e05fda2

fixes

80ce215

Update src/main/java/ch/njol/skript/conditions/CondCanSee.java

66d29ec

Co-authored-by: Moderocky <admin@moderocky.com>

Moderocky force-pushed the visibility branch from f7f33f9 to 66d29ec Compare August 18, 2024 15:50

Moderocky added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Aug 20, 2024

Moderocky assigned APickledWalrus Aug 20, 2024

Efnilite and others added 2 commits August 24, 2024 16:07

Merge branch 'dev/feature' into visibility

e08eaec

Merge branch 'dev/feature' into visibility

a2f12d1

Moderocky requested review from APickledWalrus and removed request for ShaneBeee August 30, 2024 10:50

TheLimeGlass approved these changes Aug 30, 2024

View reviewed changes

TheLimeGlass requested a review from ShaneBeee August 30, 2024 14:51

Moderocky and others added 5 commits August 30, 2024 16:35

Merge branch 'dev/feature' into visibility

34af419

Update src/test/java/org/skriptlang/skript/test/tests/syntaxes/effect…

45d7790

…s/EffEntityVisibilityTest.java Co-authored-by: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com>

Update src/test/java/org/skriptlang/skript/test/tests/syntaxes/condit…

56794b8

…ions/CondCanSeeTest.java Co-authored-by: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com>

update invisibility tag

b48bb81

Merge remote-tracking branch 'origin/visibility' into visibility

8bda88c

Merge branch 'dev/feature' into visibility

59fe469

# Conflicts: # src/main/java/ch/njol/skript/conditions/CondCanSee.java # src/main/java/ch/njol/skript/effects/EffPlayerVisibility.java # src/main/java/ch/njol/skript/expressions/ExprHiddenPlayers.java

sovdeeth merged commit d763411 into SkriptLang:dev/feature Oct 13, 2024
5 checks passed

Efnilite deleted the visibility branch October 13, 2024 15:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework entity visibility #6867

Rework entity visibility #6867

Efnilite commented Jul 5, 2024 •

edited

Loading

cheeezburga commented Jul 5, 2024

Efnilite commented Jul 5, 2024

cheeezburga commented Jul 5, 2024

sovdeeth commented Jul 5, 2024

ShaneBeee left a comment

cheeezburga left a comment

sovdeeth left a comment

sovdeeth left a comment

Efnilite commented Jul 14, 2024

sovdeeth commented Jul 18, 2024

TheLimeGlass left a comment •

edited

Loading

sovdeeth commented Oct 13, 2024

Efnilite commented Oct 13, 2024

Rework entity visibility #6867

Rework entity visibility #6867

Conversation

Efnilite commented Jul 5, 2024 • edited Loading

Description

cheeezburga commented Jul 5, 2024

Efnilite commented Jul 5, 2024

cheeezburga commented Jul 5, 2024

sovdeeth commented Jul 5, 2024

ShaneBeee left a comment

Choose a reason for hiding this comment

cheeezburga left a comment

Choose a reason for hiding this comment

sovdeeth left a comment

Choose a reason for hiding this comment

sovdeeth left a comment

Choose a reason for hiding this comment

Efnilite commented Jul 14, 2024

sovdeeth commented Jul 18, 2024

TheLimeGlass left a comment • edited Loading

Choose a reason for hiding this comment

sovdeeth commented Oct 13, 2024

Efnilite commented Oct 13, 2024

Efnilite commented Jul 5, 2024 •

edited

Loading

TheLimeGlass left a comment •

edited

Loading