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

Limit texture size #6

Merged
merged 22 commits into from
Nov 29, 2024
Merged

Limit texture size #6

merged 22 commits into from
Nov 29, 2024

Conversation

SwissalpS
Copy link
Contributor

@SwissalpS SwissalpS commented Nov 28, 2024

  • fixes metadata can get too big for server to handle #5
  • some other hidden issues.
  • cleans up code-style
  • adds luacheck
  • minimizes metadata size
  • keeps history per player for next flag
  • updates mask buttons according to selected colour (players were confused whether it was the black part or the white part that was going to be coloured)

- use same whitespace style throughout and fix some indents.
- change from minetest. to core. namespace.
- unused arguments
- shaddowed vars
- var redeclarations
- some whitespace involved in above lines
save some lines and some extra method calls.
...that slipped through the cracks on previous whitespace edit
or use the faster repeat-until-loop
fixes #5
transformation history was shared by all users and kept growing as it
was never truely reset. Every time a user
used a banner a white background was dumped on top of the
stack making it possible to crash the server through an
overflow to core.serialize() function.
@SwissalpS SwissalpS added bug Something isn't working enhancement New feature or request labels Nov 28, 2024
also changed banners.max_transformations to
banners.max_undo_levels and lowered the value substantially.
since it isn't updating everything, only
the preview and the inventory item
to reflect the currently selected colour.
@SwissalpS SwissalpS marked this pull request as ready for review November 28, 2024 14:40
@SwissalpS
Copy link
Contributor Author

I suggest squashing this merge ;)

Copy link

@BuckarooBanzay BuckarooBanzay left a comment

Choose a reason for hiding this comment

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

LGTM, do i get this right: you are just removing the oldest overlays if the number gets too high (256 currently)?

@SwissalpS
Copy link
Contributor Author

Not quite. The 256 limit is for the undo-history, which is in RAM and now per player.

The metadata stored in the item used to have the undo history of all players in it for no reason. Now the string generator works from the top of the stack downward and stops the first time it finds a background. It also skips any duplicate masks that make no difference in the resulting image.

@SwissalpS
Copy link
Contributor Author

max metadata size is now:

x = #masks
y = #longest_mask_name
z = #longest_colour_name
max_size = x * (y + z)

Of course the exact max_size is what comes out of core.serialize().

The undo-limit now opens an easter egg where players can make flags without a background. For that they have to add 256 masks that are not background so that the original background is dropped off the stack.

@SwissalpS SwissalpS mentioned this pull request Nov 29, 2024
@SwissalpS SwissalpS merged commit 8cc1c82 into master Nov 29, 2024
2 checks passed
@SwissalpS SwissalpS deleted the limitTextureSize branch November 29, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

metadata can get too big for server to handle
2 participants