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

Syncronize circle sizes #2077

Open
lynxlynxlynx opened this issue May 6, 2024 · 4 comments
Open

Syncronize circle sizes #2077

lynxlynxlynx opened this issue May 6, 2024 · 4 comments
Labels
bug research needed usually means testing in the original, to see how it behaved system: core core engine stuff system: pathfinding
Milestone

Comments

@lynxlynxlynx
Copy link
Member

As noted in one of the code TODOs (Map::GetBlockedInRadiusTile), it turns out our bigger circle sizes are indeed different than in the originals. And that they should be ellipses.

0x1208 Dragon (13 cells height in both, circle size 7):
image

0x7F04 iron golem, from the 9th level shapechange (clearly wrong circle size in our data, otherwise 5 vs 7):
image
fixed:
image

So circle sizes that we know match: 0, 2, 3
1 is not used by anything from what I can tell. Even rats and cats have 2 in bg2ee. 4 perhaps not as well (tried a nabassu, clearly a 3). 5 and 6 we don't assign to anything in bg2.

All our circle sizes:

$ awk '{print $7}' gemrb/unhardcoded/*/avatars.2da | sort | uniq -c
     15 
     96 0
     32 1
   1971 2
    157 3
     49 4
     28 5
      3 6
     45 7

Overall this means so far we only know we need to fix circle size 7 generation and to recheck iwd1/bg1/bg2 avatars.2da entries with circle size 4-6 (some we clearly have set too high). The existing cheats for cycling animation types can be used in all games to make testing faster. In EEs and maybe pst also the animation ini files can be checked to find candidates (eg. the personal_space key).

@lynxlynxlynx lynxlynxlynx added bug system: core core engine stuff research needed usually means testing in the original, to see how it behaved system: pathfinding labels May 6, 2024
@lynxlynxlynx lynxlynxlynx added this to the 0.9.4 - TBN milestone May 6, 2024
@bradallred
Copy link
Member

And that they should be ellipses.

does changing PaintSearchMap to use PlotEllipse instead of PlotCircle fix it tho? You can try playing with the initial constants for both and see what happens. I know that my initial implementation for PlotCircle generated different outcomes and I changed it to match our existing behavior.

@bradallred
Copy link
Member

this might be a helpful resource to see how the shape changes with different algorithms: https://funloop.org/post/2021-03-15-bresenham-circle-drawing-algorithm.html

@lynxlynxlynx
Copy link
Member Author

lynxlynxlynx commented May 6, 2024

Sorry, with the ellipse comment I was referring to the actual green selection circle, not the impeding approximation.
And for 13, looking at the comparison for 6, none of those generated circles match.

@bradallred
Copy link
Member

ah, that makes sense

And for 13, looking at the comparison for 6, none of those generated circles match.

There are still tweakable constants for each that can change the outcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug research needed usually means testing in the original, to see how it behaved system: core core engine stuff system: pathfinding
Projects
None yet
Development

No branches or pull requests

2 participants