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

Rename tickDelta to tickProgress #4094

Merged
merged 6 commits into from
Jan 29, 2025
Merged

Conversation

Octol1ttle
Copy link
Contributor

This PR replaces all mentions of 'tickDelta' with 'tickProgress'. For an explanation of 'tickDelta', see FabricMC/fabric-docs#173

I hesitated to make this pull request for a while. I think the name 'tickProgress' is not as good as it can be. Parchment uses 'partialTick', but some Fabricord members argued that it is too vague and it should be named 'fractionalTick' instead
I went with 'tickProgress' because it lines up with its main chunk of usages - MathHelper#lerp (MathHelper.lerp(tickProgress, prevPos, pos) sounds more obvious than MathHelper.lerp(partialTick, prevPos, pos))
If you have any suggestions for a better name, please don't be afraid to leave a comment

Copy link
Contributor

@Earthcomputer Earthcomputer left a comment

Choose a reason for hiding this comment

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

🚀

@modmuss50 modmuss50 requested a review from a team January 22, 2025 08:50
@modmuss50 modmuss50 added the refactor A PR that renames existing names. label Jan 22, 2025
@Octol1ttle
Copy link
Contributor Author

Good catch, my replace-all command was case-sensitive 😅

@NebelNidas
Copy link
Member

NebelNidas commented Jan 22, 2025

I'd also check whether parameters have been shortened from tickDelta to delta, and if there are any derivative names that need changing too (perhaps https://github.com/Octol1ttle/yarn/blob/02cf0a78eec8bdcc966c67baa42f6c3aa93d94cd/mappings/net/minecraft/entity/passive/CamelBrain.mapping#L30? Would have to look at the code)

@Octol1ttle
Copy link
Contributor Author

I'd also check whether parameters have been shortened from tickDelta to delta, and if there are any derivative names that need changing too (perhaps https://github.com/Octol1ttle/yarn/blob/02cf0a78eec8bdcc966c67baa42f6c3aa93d94cd/mappings/net/minecraft/entity/passive/CamelBrain.mapping#L30? Would have to look at the code)

I've checked what I could and replaced it. Note that in Screens and associated classes the delta is actually deltaTicks - the time difference between two frames, in ticks

I recommend that the maintainers review this PR by checking each commit separately to not get lost too much

@Octol1ttle Octol1ttle changed the base branch from 25w03a to 25w04a January 25, 2025 17:01
Copy link
Contributor

@Shnupbups Shnupbups left a comment

Choose a reason for hiding this comment

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

I think this is as good a time as any to make this change - gonna wait for another approval before merge though

@Shnupbups Shnupbups requested review from haykam821, apple502j and a team January 29, 2025 08:26
@Shnupbups Shnupbups merged commit 5535cf4 into FabricMC:25w04a Jan 29, 2025
2 checks passed
@Octol1ttle Octol1ttle deleted the tickprogress branch January 29, 2025 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor A PR that renames existing names.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants