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 out-of-bounds textures in [combine #27

Merged
merged 1 commit into from
Apr 19, 2024
Merged

Conversation

OgelGames
Copy link
Contributor

Fixes #24 by preventing character and fill textures from being added past the end of each line of text.

@BuckarooBanzay BuckarooBanzay added enhancement New feature or request bugfix fixes a bug and removed enhancement New feature or request labels Apr 4, 2024
Copy link
Member

@Niklp09 Niklp09 left a comment

Choose a reason for hiding this comment

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

Works, I prefer this over #25.

@appgurueu
Copy link

Why? I'm not convinced keeping the fill_line hack is a good idea; I don't see why it would be needed for the signs_lib functionality, and it surely is inefficient.

To me, the other PR cuts down on the mess and dirty hacks, while this PR adds further complexity. Assuming both ultimately implement the same features correctly, why prefer this PR?

I'd also be interested in hearing what @tenplus1 thinks.

@tenplus1
Copy link
Contributor

tenplus1 commented Apr 5, 2024

@appgurueu is corrent, while this may work it adds further complexity and sticks with the old masking method of sign creation that was hacky to begin with. The updated method in #25 only has to draw the coloured text itself which results in faster creation of signage.

@OgelGames
Copy link
Contributor Author

Neither method is significantly better or worse than the other, and it's definitely not a hack to use masking to color the text.

The biggest difference is that this PR fixes the cause of the out-of-bounds textures, preventing overflowing text from being added to the texture.

@OgelGames
Copy link
Contributor Author

It's been 2 weeks, merging this.

@OgelGames OgelGames merged commit 4fb53b6 into master Apr 19, 2024
2 checks passed
@OgelGames OgelGames deleted the out-of-bounds-fix branch April 19, 2024 03:44
OgelGames added a commit that referenced this pull request Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

out-of-bounds combine textures
5 participants