Skip to content

ExprMidpoint#7888

Merged
sovdeeth merged 21 commits intoSkriptLang:dev/featurefrom
Absolutionism:dev/ExprCenterLocations
Oct 1, 2025
Merged

ExprMidpoint#7888
sovdeeth merged 21 commits intoSkriptLang:dev/featurefrom
Absolutionism:dev/ExprCenterLocations

Conversation

@Absolutionism
Copy link
Contributor

@Absolutionism Absolutionism commented May 22, 2025

Problem

Currently is no quick and easy route of getting the center location from two locations.
The only current way I know to achieve this, is
set {_center} to {_loc1} ~ ((vector from {_loc1} to {_loc2}) / vector(2,2,2))

Solution

Adds ExprMidpoint that does this.

Testing Completed

ExprMidpoint.sk


Completes: none
Related: none

@Absolutionism Absolutionism requested a review from a team as a code owner May 22, 2025 17:28
@Absolutionism Absolutionism requested review from cheeezburga and erenkarakal and removed request for a team May 22, 2025 17:28
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label May 22, 2025
@ShaneBeee
Copy link
Contributor

Just going to throw my 2 cents in here.
This in my opinion seems like bloat.

Skript should focus on the language and the Bukkit API itself.

Skript shouldn't be putting on focus on doing math that a Skript user could do themselves.
This feels more like something meant for an addon.

Again, I repeat, just my opinion.

@sovdeeth
Copy link
Member

sovdeeth commented May 22, 2025

I'm kind of ambivalent on this, since it seems like a really easy thing to do with existing tools. It's a bit clunky since you have to get a vector or do math on x/y/z, though, so i see the appeal of a simpler manner. I don't mind it either way.

That said, I'm strongly against calling it the center point. I think that muddles the waters between the existing center expression (and could cause syntax conflicts as is currently written). I would stick entirely to the term midpoint for this.

I'd like to comment that the pr template does ask you to argue why the given problem needs solving, and I think just the fact that no QOL expression exists is not reason enough. I'd like to see some arguments as to why it needs to be added and why it's better than whatever you can currently do.

This is also missing any documentation of testing completed.

@Absolutionism
Copy link
Contributor Author

So you want only [the] midpoint (between|of) %location% and %location% + from ... to ...

@sovdeeth
Copy link
Member

sovdeeth commented May 22, 2025

So you want only [the] midpoint (between|of) %location% and %location% + from ... to ...

well midpoint from x to y doesn't make much sense, but the between/of seems fine, yes
(also fyi you can divide the vector by 2 directly, instead of using a vector of 2s)

@sovdeeth sovdeeth added the feature Pull request adding a new feature. label May 22, 2025
@Absolutionism Absolutionism requested a review from sovdeeth May 22, 2025 20:58
@Absolutionism Absolutionism changed the title ExprCenterLocations ExprMidpointLocations May 24, 2025
@Absolutionism Absolutionism requested a review from sovdeeth May 25, 2025 14:02
@skriptlang-automation skriptlang-automation bot removed the needs reviews A PR that needs additional reviews label May 28, 2025
@erenkarakal
Copy link
Member

checks failing

@erenkarakal erenkarakal moved this from Awaiting Merge to In Review in 2.12 Releases Jun 5, 2025
@erenkarakal erenkarakal moved this from In Review to Awaiting Merge in 2.12 Releases Jun 5, 2025
@Absolutionism Absolutionism changed the title ExprMidpointLocations ExprMidpoint Jun 5, 2025
Copy link
Member

@Efnilite Efnilite left a comment

Choose a reason for hiding this comment

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

tests failing

@github-project-automation github-project-automation bot moved this from Awaiting Merge to In Review in 2.12 Releases Jun 8, 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 Jun 8, 2025
@Absolutionism Absolutionism requested a review from Efnilite June 13, 2025 15:23
Copy link
Member

@Efnilite Efnilite left a comment

Choose a reason for hiding this comment

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

still failing 😬

@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 Oct 1, 2025
@sovdeeth sovdeeth merged commit 50c1b6b into SkriptLang:dev/feature Oct 1, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done - Awaiting Release in 2.13 Releases Oct 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 Oct 1, 2025
@sovdeeth sovdeeth moved this from Done - Awaiting Release to Done - Released in 2.13 Releases Oct 15, 2025
erenkarakal pushed a commit to erenkarakal/Skript that referenced this pull request Nov 26, 2025
* Initial Commit

* Update ExprCenterLocations.sk

* Update ExprCenterLocations.java

* Requested Changes

* Update ExprMidpointLocations.java

* Allow Vectors

* Test + Error Update

* Update ExprMidpoint.java

* Update ExprMidpoint.java

* Update ExprMidpoint.java

* Runtime Errors
erenkarakal pushed a commit to erenkarakal/Skript that referenced this pull request Nov 26, 2025
* Initial Commit

* Update ExprCenterLocations.sk

* Update ExprCenterLocations.java

* Requested Changes

* Update ExprMidpointLocations.java

* Allow Vectors

* Test + Error Update

* Update ExprMidpoint.java

* Update ExprMidpoint.java

* Update ExprMidpoint.java

* Runtime Errors
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. feature Pull request adding a new feature.

Projects

No open projects
Status: Done - Released

Development

Successfully merging this pull request may close these issues.

8 participants

Comments