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

Target block not working as expected due to caching #4341

Closed
1 task done
LoganLilypad opened this issue Sep 25, 2021 · 1 comment
Closed
1 task done

Target block not working as expected due to caching #4341

LoganLilypad opened this issue Sep 25, 2021 · 1 comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: low Issues that are not harmful to the experience but are related to useful changes or additions.

Comments

@LoganLilypad
Copy link

Skript/Server Version

[16:42:53 INFO]: [Skript] Skript's aliases can be found here: https://github.com/SkriptLang/skript-aliases
[16:42:53 INFO]: [Skript] Skript's documentation can be found here: https://skriptlang.github.io/Skript
[16:42:53 INFO]: [Skript] Server Version: git-Paper-124 (MC: 1.17.1)
[16:42:53 INFO]: [Skript] Skript Version: 2.6-beta3
[16:42:53 INFO]: [Skript] Installed Skript Addons:
[16:42:53 INFO]: [Skript]  - Skirt v1.2.3
[16:42:53 INFO]: [Skript]  - SkBee v1.11.2 (https://github.com/ShaneBeee/SkBee)

Bug Description

Attempting to get the player's target block in a loop (specifically the distance, but that's irrelevant to the whole issue) will not update in a simple send effect or any expression when the player's rotation changes. I've been informed that this is likely caused because of Skript caching things such as blocks in the previous world tick in the main world only (???) if the world time is no longer ticking (i.e., daylight cycle is off). The two solutions I have found are to either enable the daylight cycle or to change target block to new target block or actual target block.

Expected Behavior

Expected behavior is to return the actual target block in this case and not one from cache.

Steps to Reproduce

Simply using a snippet like this and running the command with doDaylightCycle off in the main world and proceeding to look around reveals that the distance will not change through rotation (looking at blocks around you at different distances), but if you physically move around it will change, but it will still treat the target block as the one you originally looked at. Either changing doDaylightCycle to true or prefixing target block with new or actual will make it perform what is expected.

command /target:
    trigger:
        loop 200 times:
            send "&a%distance between player and target block%"
            wait 1 tick

Errors or Screenshots

No response

Other

Hopefully the way Skript caches things is fixed, I bet I'm not the first to encounter such behavior and having the actual part of target block not documented may be an issue.

Agreement

  • I have read the guidelines above and confirm I am following them with this report.
@TPGamesNL
Copy link
Member

ExprTargetedBlock has a cache:

v1.3
'Targeted block' is now stored during an event, resulting in faster execution and no unexpected results when changing any blocks in the player's LOS. The current targeted block can still be accessed with 'future targeted block' or 'targeted block will be/will not be/etc.'.

v1.4
Added expression 'actual targeted block of '

This cache has a validity check, which partly uses the world time, which is why doDayLightCycle plays a role here.

From the commit message, the cache was added because it led to unexpected results when changes in a players line of sight occured, whatever that means.
I think it'd be better to remove the cache entirely, and deprecate the second feature (which overrides the cache).

If for some reason the cache should be kept, we do need to improve it, e.g. by making it rely on a location instead of a Player instance, and by making it rely on a reliable time instead of the world time, e.g. a simple task that runs every tick that adds 1 to a counter.

@TPGamesNL TPGamesNL added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: low Issues that are not harmful to the experience but are related to useful changes or additions. labels Sep 26, 2021
@TPGamesNL TPGamesNL added PR available Issues which have a yet-to-be merged PR resolving it completed The issue has been fully resolved and the change will be in the next Skript update. and removed PR available Issues which have a yet-to-be merged PR resolving it labels Oct 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: low Issues that are not harmful to the experience but are related to useful changes or additions.
Projects
None yet
Development

No branches or pull requests

3 participants