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

[Bug]: Readme: Trample formula is confusing #7

Closed
mk-pmb opened this issue Aug 30, 2024 · 5 comments
Closed

[Bug]: Readme: Trample formula is confusing #7

mk-pmb opened this issue Aug 30, 2024 · 5 comments
Labels
documentation Improvements or additions to documentation

Comments

@mk-pmb
Copy link

mk-pmb commented Aug 30, 2024

Describe the Bug

In the current notation width * width * height, it's unclear whether you meant width² or maybe it's a mistake and you meant width × length × height.

Steps to Reproduce

Read the readme, encounter the formula.

Expected Behavior

The formula should either include length, or be followed by an explanation of why length is not used.

Screenshots

No response

Fabric Version

No response

Mod Version

none (yet)

Log Output

No response

Additional Context

No response

@BVengo
Copy link
Owner

BVengo commented Aug 31, 2024

Cheers MK,

I'll make that clear in the README.

That formula is directly from the code, and they use the squared width. I'm not entirely sure what their reasoning is for not using length either!

@BVengo BVengo added the documentation Improvements or additions to documentation label Aug 31, 2024
@BVengo BVengo closed this as completed in a7a6f2d Nov 29, 2024
@mk-pmb
Copy link
Author

mk-pmb commented Nov 29, 2024

Not sure if there was a misunderstanding, but I cannot see the expected behavior in this patch.
Using ² in code format may confuse people, thinking it may be a footnote, especially when multiplication is expressed with * rather than ·. I'd suggest ^2 for clarification.

@BVengo
Copy link
Owner

BVengo commented Nov 29, 2024

I think at this stage we're being pedantic 🙂 I honestly think the original width * width * height is most clear, but I've made the change to remove any question about correctness.

Using ² in code format may confuse people, thinking it may be a footnote, especially when multiplication is expressed with * rather than ·. I'd suggest ^2 for clarification.

Neither superscript nor carets are valid Java code - I picked the nicer looking format for documentation purposes, and wrapped it in a code block since using mathjax is overkill.

The formula should either include length, or be followed by an explanation of why length is not used.

No explanation is provided in the MC code. I would think length is correct, but it is what it is.

At the end of the day, I'm only including the equation alongside the description for those who are curious. I don't believe the calculation impact the majority of people, and those who care can work it out pretty quickly.

@mk-pmb
Copy link
Author

mk-pmb commented Dec 1, 2024

nor carets are valid Java code

Thanks, I didn't know that. In that case, yeah, superscript is the best option IMO.

No explanation is provided in the MC code.

Then the explanation to readers of this repo should be something like "Minecraft code uses square width and disregards length. I couldn't find a reason for why." That makes it clear that it's not a mistake of yours, and maybe even inspires people to explain Mojang's reasoning.

@BVengo
Copy link
Owner

BVengo commented Dec 1, 2024

My purpose is to convey the information succinctly, not to attempt explaining the base code behind it. I think if anyone really wants to go that in depth, my discord is always open and available.

I'll properly shut down this issue now, since I won't be making further changes for now. Thanks for all your thoughts on the matter though.

Repository owner locked and limited conversation to collaborators Dec 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants