Skip to content

Runtime error when calculating distance between different worlds#8196

Merged
sovdeeth merged 7 commits intoSkriptLang:dev/featurefrom
Chrissblock99:distance-change
Sep 20, 2025
Merged

Runtime error when calculating distance between different worlds#8196
sovdeeth merged 7 commits intoSkriptLang:dev/featurefrom
Chrissblock99:distance-change

Conversation

@Chrissblock99
Copy link
Copy Markdown
Contributor

Problem

Below is a code sample which checks whether some location is near the word origin

if distance between {_loc} and location(0,0,0,world "world") > 1:
    exit

The problem is that this check fails (doesn't exit) when the location is in a different world, because the distance currently returns in that case. This kind of makes sense (the distance between two worlds doesn't exist), however it works inconsistently as seen in the above example.
Moreover you can invert the logic ( a > b is equivalent to !(a <= b) ), but end up with different behavior.

Solution

Changing the returned value from to infinity results in more consistent behavior.

if distance between {_loc} and location(0,0,0,world "world") > 1:
    exit

Here infinity is larger than one, so the function exits correctly.
Additionally infinity better preserves some underlying mathematical properties which are discussed in the discord link under Related

Testing Completed

I tested that this returns the correct value by continuously printing the distance between myself and the world origin. It worked in the overworld and returned infinity when I went to the nether.
The unit tests all ran successfully (I ran the gradle quickTest action).

Supporting Information

This can change the behavior of existing skripts when they compare locations from different worlds, especially when they specifically account for the result being .


Completes: none
Related: https://discord.com/channels/135877399391764480/836220422223036467/1418191935310528517

@Chrissblock99 Chrissblock99 requested a review from a team as a code owner September 18, 2025 19:45
@Chrissblock99 Chrissblock99 requested review from Pesekjak and TheMug06 and removed request for a team September 18, 2025 19:45
@Fusezion
Copy link
Copy Markdown
Contributor

Personally I'm against this change, with how skript acts in most cases like these it makes sense for it to return nothing
if anything Skript should probably prioritize a runtime error for it being in two different worlds.

@sovdeeth sovdeeth added enhancement Feature request, an issue about something that could be improved, or a PR improving something. up for debate When the decision is yet to be debated on the issue in question breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) labels Sep 18, 2025
@Chrissblock99
Copy link
Copy Markdown
Contributor Author

Someone in the skunity discord also brought up that this would hard crash if someone does loop distance / x times. Not really sure whats best here. There was prior discussion in the skunity discord, but a runtime error was actually not brought up.

@sovdeeth
Copy link
Copy Markdown
Member

I think this change is fine, but given the change to behavior with things like math, it may just be better to tell people to check the worlds of two locations if they think they may be different. A runtime error would help with that (specifically for differing worlds, not null values). No strong feelings either way.

@Chrissblock99
Copy link
Copy Markdown
Contributor Author

With the possibility of a hard crash when returning infinity, instead of having it error and tell them to check manually, I think the second option might be better. Additionally things could stay as is, which is more seamless, but can trip you up badly. (That's the original reason I wanted some change)

@Chrissblock99
Copy link
Copy Markdown
Contributor Author

I would have to ask how to properly throw a runtime error in skript though.

@sovdeeth
Copy link
Copy Markdown
Member

you should just be able to call error("text") within the get() method

@Chrissblock99
Copy link
Copy Markdown
Contributor Author

you should just be able to call error("text") within the get() method

Should I just do that in this PR?

@sovdeeth
Copy link
Copy Markdown
Member

Sure, if ya want

@Chrissblock99
Copy link
Copy Markdown
Contributor Author

When I call error("text") I still have to return a value after it. (If I don't return we run into the IllegalArgumentException from the APIs distance calculation, which is quite ugly) I just return infinity right now, but that can still hard crash.

I had someone argue in my dms that a hard crash would already happen if players went sufficiently far apart. Just putting that out there, not sure what to do with it.

@sovdeeth
Copy link
Copy Markdown
Member

When I call error("text") I still have to return a value after it. (If I don't return we run into the IllegalArgumentException from the APIs distance calculation, which is quite ugly) I just return infinity right now, but that can still hard crash.

I had someone argue in my dms that a hard crash would already happen if players went sufficiently far apart. Just putting that out there, not sure what to do with it.

you return null, to maintain existing behavior.

@Chrissblock99
Copy link
Copy Markdown
Contributor Author

Honestly I am slightly unsatisfied with just a runtime error, but I have the changes ready to go. I ran the test suite locally again and tested on a server (It gives none and an error when in different worlds (wow)). I am kind of asking for confirmation on this being the wanted result.

@sovdeeth
Copy link
Copy Markdown
Member

Yes, that seems like what should be done.

@sovdeeth sovdeeth changed the title Return infinity when calculating distance between different worlds Runtime error when calculating distance between different worlds Sep 18, 2025
@sovdeeth sovdeeth removed the breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) label Sep 18, 2025
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Sep 18, 2025
@sovdeeth sovdeeth changed the base branch from master to dev/feature September 18, 2025 22:27
sovdeeth and others added 2 commits September 18, 2025 15:27
suggestion made by sovdeeth

Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>
@Chrissblock99
Copy link
Copy Markdown
Contributor Author

Oh god i merged bogus code, because I thought the dots where just some end of line width thing i couldn't unwrap. Whoops

suggestion by sovdeeth
Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>
@sovdeeth
Copy link
Copy Markdown
Member

Ah just missing an import for Classes and it should be good after that

@sovdeeth sovdeeth removed the up for debate When the decision is yet to be debated on the issue in question label Sep 18, 2025
Copy link
Copy Markdown
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

wonderful; thank you for the pr, as well as all the analysis around infinity/null behavior!

@skriptlang-automation skriptlang-automation bot added feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. and removed needs reviews A PR that needs additional reviews labels Sep 20, 2025
@sovdeeth sovdeeth merged commit 2b4b50d into SkriptLang:dev/feature Sep 20, 2025
5 checks passed
@skriptlang-automation skriptlang-automation bot added completed The issue has been fully resolved and the change will be in the next Skript update. and removed feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. labels Sep 20, 2025
erenkarakal pushed a commit to erenkarakal/Skript that referenced this pull request Nov 26, 2025
…iptLang#8196)

* Return infinity when calculating distance between different worlds

* Throw a runtime error when calculating distance between different worlds

* add world names to runtime error message

suggestion made by sovdeeth

Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>

* fix runtime error message

suggestion by sovdeeth

* Update runtime error message

Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>

* Update ExprDistance.java
erenkarakal pushed a commit to erenkarakal/Skript that referenced this pull request Nov 26, 2025
…iptLang#8196)

* Return infinity when calculating distance between different worlds

* Throw a runtime error when calculating distance between different worlds

* add world names to runtime error message

suggestion made by sovdeeth

Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>

* fix runtime error message

suggestion by sovdeeth

* Update runtime error message

Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>

* Update ExprDistance.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants