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

Fix issues regarding entity movement methods #702

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

BenCheung0422
Copy link
Member

@BenCheung0422 BenCheung0422 commented Oct 14, 2024

Main issue fixed:

Entity#move still returned true even when not moved as either x or y delta is zero. The boolean result should indicate whether the entity is moved (false whenever completely not moved), when any non-zero argument is passed. (This issue was found when doing #592)
Furthermore, the line

if (d == 0) return true; // Was not stopped

was kept just because the original method contains the check, but was for when both x and y delta are zero. The code is then checked somehow to be logically incorrect, and the actual logically correct line already exists in #move as

if (Updater.saving || (xd == 0 && yd == 0)) return true; // Pretend that it kept moving

Minor (potential) issues in FireSpark (mostly fixed):

  • Entity#x and Entity#y used in #moveX and #moveY were not taken into account when calculating distance of movement, which may lead to potential increasing gap between "real coordinates" and "virutal coordinates".
  • Superclass #move is not called, which may lead to inconsistent operations between overriding code and original method.
  • Suppose this made just negligible effect to the current system, this issue is not fixed to simplify the situation. As real movement handled by #move depends on integral real coordinates, it would still return as "not moved" when the movement (from virtual double coordinates) made less than a unit/"pixel" of movement (no "movement" in integral floored coordinates), leading to unexpected result. However, at the moment, no obvious impact is found.

@BenCheung0422 BenCheung0422 added Bug Something that shouldn't happen. Code Quality Issue related to code quality/coding standard. World system Issue related to game world system/mechanics. labels Oct 14, 2024
@Litorom Litorom merged commit 1bc74aa into MinicraftPlus:main Oct 31, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something that shouldn't happen. Code Quality Issue related to code quality/coding standard. World system Issue related to game world system/mechanics.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants