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

Debug lines for y-sorting #230

Merged
merged 2 commits into from
Aug 14, 2022
Merged

Debug lines for y-sorting #230

merged 2 commits into from
Aug 14, 2022

Conversation

edgarssilva
Copy link
Collaborator

A PR to draw debug lines for the y-sorting that enables easy debugging for fixing current y-sorting problems.

@erlend-sh
Copy link
Member

This is a visual thing, right? Could you please share a screenshot of the change in action?

@edgarssilva
Copy link
Collaborator Author

This is a visual thing, right? Could you please share a screenshot of the change in action?

It's still in progress, it's just basically a line at the bottom of each sprite representing the point at which sprites appear behind each other. Just to help debug that value.

When It's ready I can get a screenshot.

src/ui/debug_tools.rs Outdated Show resolved Hide resolved
@edgarssilva
Copy link
Collaborator Author

So after adding the debug lines it was easy to tell that the problem is caused because of the different padding between sprite sheets. I think adding a fighter_size property to the fighters is a good idea and can be beneficial later on.

Fix y-sort by adding size to the fighters.
@edgarssilva
Copy link
Collaborator Author

Ok so I just finished up everything should be working fine now.
One thing we should be careful of is that skins of the same fighter should be packed into a sheet the same way.
Right now we have one case where this happens but it's a very small one that won't be noticeable. But to keep in mind in the future.

different_padding

@edgarssilva edgarssilva marked this pull request as ready for review August 13, 2022 20:50
Copy link
Member

@zicklag zicklag left a comment

Choose a reason for hiding this comment

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

Haven't tested but the code looks good. 👍

@edgarssilva
Copy link
Collaborator Author

@erlend-sh Here you go.

image

@edgarssilva
Copy link
Collaborator Author

bors r+

@bors bors bot merged commit 918b829 into fishfolk:master Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants