Skip to content

Push X towards Y #7950

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

Merged
merged 5 commits into from
Jul 1, 2025
Merged

Conversation

sovdeeth
Copy link
Member

Problem

It's quite common for users to want to push one entity towards another, or towards a specific location. This isn't very hard, it's push {_X} along (vector from {_x} to {_y}), but it's not immediately obvious. The main reason is that the pattern is push %entities% [along] %direction% and it's not explained that a vector is also a direction.

This also has another weakness: pushing multiple entities towards the same point. This requires a loop over the entities to calculate the vector for each one.

Solution

I've added syntax to make it more intuitive and efficient to do this: push %entities% (away from|towards) %location%. Internally, it acts the same as push {_X} along (vector from {_x} to {_y}), it's just simpler for the user and more accessible for those unfamiliar with directions/vector interplay.

I've made two possibly controversial choices that I'd like opinions on:

  1. This mirrors the previous solution by not normalizing the direction vector if no speed is applied. This is faster and prevents double-normalization when speed is provided, but it also may be confusing for new users who go flying off into outerspace when their target location is 20 blocks away.
  2. I have treated the location as if it is always within the same world as the entity being pushed towards it. This makes the code a lot cleaner and removes some annoying checks. I'm of the opinion that pushing towards a different world's location is a really rare mistake to make, and that if it is made, treating it as if it's the same world isn't likely to be very confusing. The only case I can see this being an issue is if someone tries to rely on it not working to only push people in the same world, which sounds extremely uncommon.

Testing Completed

Added EffPush.sk

Supporting Information

I am counting this as completing #1671, as though it isn't a direction, it is fulfilling the requested behavior.
I didn't want to make this a direction because I feel directions are already a bit bloated and hard to understand, and that it wouldn't really reduce the confusion around how to do this task.


Completes: #1671
Related: none

@sovdeeth sovdeeth requested a review from a team as a code owner June 15, 2025 02:35
@sovdeeth sovdeeth added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Jun 15, 2025
@sovdeeth sovdeeth requested review from cheeezburga and Burbulinis and removed request for a team June 15, 2025 02:35
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Jun 15, 2025
@sovdeeth sovdeeth moved this to In Review in 2.12 Release Jun 15, 2025
@sovdeeth sovdeeth linked an issue Jun 15, 2025 that may be closed by this pull request
@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 Jun 15, 2025
@sovdeeth sovdeeth moved this from In Review to Awaiting Merge in 2.12 Release Jun 19, 2025
@sovdeeth sovdeeth marked this pull request as draft June 19, 2025 23:32
@skriptlang-automation skriptlang-automation bot removed the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Jun 19, 2025
@sovdeeth sovdeeth marked this pull request as ready for review June 19, 2025 23:37
@skriptlang-automation skriptlang-automation bot added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Jun 19, 2025
@github-project-automation github-project-automation bot moved this from Awaiting Merge to In Review in 2.12 Release Jul 1, 2025
@skriptlang-automation skriptlang-automation bot removed the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Jul 1, 2025
@sovdeeth sovdeeth requested a review from APickledWalrus July 1, 2025 01:29
@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.12 Release Jul 1, 2025
@skriptlang-automation skriptlang-automation bot added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Jul 1, 2025
@APickledWalrus APickledWalrus merged commit 80ed254 into SkriptLang:dev/feature Jul 1, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from Awaiting Merge to Done in 2.12 Release Jul 1, 2025
@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 Jul 1, 2025
Burbulinis pushed a commit to Burbulinis/Skript that referenced this pull request Jul 9, 2025
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
Status: Done
Development

Successfully merging this pull request may close these issues.

Add 'away from' direction
4 participants