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.

Sign up for GitHub

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

Jump to bottom

Midnight happenings #458

Merged
merged 3 commits into from
Jul 5, 2023
Merged

Midnight happenings #458

merged 3 commits into from
Jul 5, 2023

Conversation

EnnuiL
Copy link
Contributor

@EnnuiL EnnuiL commented Jul 2, 2023

midnight happenings

@EnnuiL EnnuiL added t: new adds new mappings v: release targets a release version of minecraft s: small PRs with less than 200 lines labels Jul 2, 2023
@ix0rai ix0rai self-requested a review July 2, 2023 04:21
@supersaiyansubtlety supersaiyansubtlety added final-comment-period is approved and will soon be merged if no issues are raised and removed final-comment-period is approved and will soon be merged if no issues are raised labels Jul 2, 2023
@EnnuiL EnnuiL added the final-comment-period is approved and will soon be merged if no issues are raised label Jul 2, 2023
ix0rai
ix0rai requested changes Jul 2, 2023
View reviewed changes
Copy link
Member

@ix0rai ix0rai left a comment

Choose a reason for hiding this comment

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

I'm not a big fan of the word order on velocityXYZ parameters, I'd prefer we went with the natural xyzVelocity

@EnnuiL
Copy link
Contributor Author

EnnuiL commented Jul 2, 2023

I'm not a big fan of the word order on velocityXYZ parameters, I'd prefer we went with the natural xyzVelocity

The problem is that it goes against the convention about that

@ix0rai
Copy link
Member

ix0rai commented Jul 2, 2023 •
edited
Loading

I'd argue that xyzVelocity fits the convention better -- you wouldn't say "this updates the velocity X of the entity to 5 blocks per second", the more natural sentence is "updates the x velocity"

@EnnuiL
Copy link
Contributor Author

EnnuiL commented Jul 2, 2023

I'd argue that xyzVelocity fits the convention better -- you wouldn't say "this updates the velocity X of the entity to 5 blocks per second", the more natural sentence is "updates the x velocity"

I'd argue that if you think of it as "velocity's X", things end making more sense; But oh well, I feel like a convention update/mass refactor would be a separate PR

@ix0rai
Copy link
Member

ix0rai commented Jul 2, 2023

ok, I'll put it on my todo list :p

@ix0rai ix0rai mentioned this pull request Jul 2, 2023
Closed
@ix0rai ix0rai merged commit 94d92bd into QuiltMC:1.20.1 Jul 5, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@supersaiyansubtlety supersaiyansubtlety supersaiyansubtlety approved these changes

@IotaBread IotaBread IotaBread approved these changes

@ix0rai ix0rai ix0rai approved these changes

Assignees
No one assigned
Labels
final-comment-period is approved and will soon be merged if no issues are raised s: small PRs with less than 200 lines t: new adds new mappings v: release targets a release version of minecraft
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@EnnuiL @ix0rai @supersaiyansubtlety @IotaBread