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

Prefer CoordsDirectionDelta on footpath_update_path_wide_flags() #10574

Open
wants to merge 3 commits into
base: develop
from

Conversation

@tupaschoal
Copy link
Contributor

tupaschoal commented Jan 15, 2020

I recommend looking at each commit, to see what's best.

  • On the first one, I just started using the CoordsDirectionDelta
  • On the second one, I simplified it into a loop
@tupaschoal tupaschoal force-pushed the tupaschoal:footpath-direction-delta branch from deafc0e to 23e1a52 Jan 15, 2020
@duncanspumpkin

This comment has been minimized.

Copy link
Contributor

duncanspumpkin commented Jan 15, 2020

I don't think this is quite right. This should be doing a look around every adjacent tile. I would expect the loop to be 1->8 and be: pos = footpathPos + CoordsDirectionDelta[direction]
Then the wide check.
The double addition looks incorrect.

Edit: Nope just me being unable to read.

pathList[i++] = footpath_can_be_wide(pathPos, height);
pathPos += CoordsDirectionDelta[direction];
direction = direction_next(direction);
}

This comment has been minimized.

Copy link
@duncanspumpkin

duncanspumpkin Jan 15, 2020

Contributor

This likely works but we could do it simpler by having direction being 0->7. And the pos being a sum of the base footpathPos plus the Delta at the specified direction. It would be a different layout of the array though. Will need a little think to check if that is an issue.

This comment has been minimized.

Copy link
@tupaschoal

tupaschoal Jan 18, 2020

Author Contributor

For reference, this is what the delta looks like at each position:

[0] = {-32, -32}
[1] = {-32, 0}
[2] = {-32, 32}
[3] = {0, 32}
[4] = {32, 32}
[5] = {32, 0}
[6] = {32, -32}
[7] = {0, -32}

This comment has been minimized.

Copy link
@tupaschoal

tupaschoal Jan 18, 2020

Author Contributor

Which translates to:

[0] = footpathPos + CoordsDirectionDelta[7];
[1] = footpathPos + CoordsDirectionDelta[0];
[2] = footpathPos + CoordsDirectionDelta[4];
[3] = footpathPos + CoordsDirectionDelta[1];
[4] = footpathPos + CoordsDirectionDelta[5];
[5] = footpathPos + CoordsDirectionDelta[2];
[6] = footpathPos + CoordsDirectionDelta[6];
[7] = footpathPos + CoordsDirectionDelta[3];

This comment has been minimized.

Copy link
@tupaschoal

tupaschoal Jan 18, 2020

Author Contributor

I've simplified it on the latest commit using the conversions above, please double check @duncanspumpkin .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.