Fix out-of-bounds in 'combine' texture modifier #25
Conversation
Co-authored-by: tenplus1 <[email protected]>
|
@Niklp09 - Thanks for the assist :) I find that it generates signs quicker this way although anyone using their own custom font sets will need to use the convert.sh I've included to negate the image files so they work with the new setup. |
agree on this, just skim'd the pr tho |
|
FWIW, you could "negate" the image files using the I have to say though: The current media files seem like an ugly hack. The actual glyph is transparent, whereas the background is solid? Hence the approach of this PR - to invert alpha - makes sense. If maintaining backwards compat with "custom fonts" - without running a script - is desired, you could use IMO, it would make the most sense to have white, opaque glyphs on transparent background. Then you could also use |
|
@appgurueu - I did try to [invert media but it never worked properly on transparent areas and the unicode textures ended up with artifacts, so in the end editing the files had to be done by hand or script. |
|
If it worries anyone how the changes affect signs_lib, check out Xanadu server, we've been using it since the commit and it's working well. |
|
I wouldn't mind the texture changes much. However, please at least run something like pngcrush - the new textures are much larger than they need to be. (You could also consider converting the textures to TGA to shave off some more bytes, or packing them up in a tilesheet.) |
|
@appgurueu - Run optipng on every texture, please find attached the final mod in .zip form. |
|
Commited to the branch, sorry for the delay. |
|
Anything stopping this? |
|
After some trial and error (the code is terrible to read), I figured out a simpler way to fix the issue: #27 |
| c = c or "0" | ||
| local tex = { } | ||
| for xx = 0, math.max(0, w), colorbgw do | ||
| table.insert(tex, (":%d,%d=signs_lib_color_"..font_size.."px_%s.png"):format(x + xx, y, c)) |
There was a problem hiding this comment.
Aren't the signs_lib_color_* files unused now that you switched this to a color lookup table & ^[colorize? Can't you remove them now (or is something else using them)?
There was a problem hiding this comment.
I only kept them around incase any other mod was using them, a compatibility thing mostly.
|
I just tested this and it doesn't fix the issue, there are still out-of-bounds textures when there is text overflow. |
|
On further inspection the signs_lib_color_(pxsize)_n.png files are required for the sign to display properly, but I have removed the colours not in use. I also applied the size changes from the OgelGames pull request into this one which works a little better, so here is the latest mod update attached :) |
|
Merged #27, closing this. |
|
@OgelGames - Could you re-open this as a working alternative. |
|
For now restored branch if someone is still working with it. Related issue was reopened so I guess meta discussion could go there. |
As I noted previously, this PR doesn't work either. I think a lot more is needed to fix it properly. |
|
cross posting frankly replacing colorized textures with |
|
If there was a good specification (and tests?) such that we could ensure low risk of breaking compatibility, I would be inclined to suggest a rewrite of this part of the logic: Generating text strings via Is there a good specification though? |
How so ? I've been using this on Xanadu and signs work fine. |
It doesn't fix the out-of-bounds errors when the text overflows past the edge of the sign. |
Fixes #24
TenPlus1 asked me to create this PR since github doesn't let him.
Tested, and works. But I'm not sure if this is the best approach 🤔 .