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

Make digistuff touchscreen be able to show raw formspecs #60

Closed
wants to merge 2 commits into from

Conversation

TheEt1234
Copy link

@TheEt1234 TheEt1234 commented Jun 25, 2024

I've added 2 ways for the digistuff touchscreen to receive raw formspecs (so like the old digistuff advanced touchscreen):

1 - the set command
Example:

{
    command = "set",
    formspec = "size[12,12]label[0,0;Hi world]" -- overrides everything, when set to "" it will stop overriding
}

2 - the add command
Example:

{
    command = "add",
    element = "formspec",
    formspec = "label[0,0;Hi!]" -- doesn't override anything, just adds a label
}

Why

there is a potential performance improvement when doing something really silly like rendering a fractal out of small images

And also allows you to send unsupported elements like scroll containers, thats like the main reason i made this, now that i think about it, i should've probably just added those in but whatever, this will keep up with the future

@TheEt1234 TheEt1234 marked this pull request as ready for review June 25, 2024 19:37
@wsor4035
Copy link

not exactly sure if i support this for two reasons

  1. formspecs suck, and the engine probably eventually will replace them. i dont want to have to support them forever
  2. given formspecs are client side, (citation needed) - with malformed formspecs one could crash clients at one point in the past, much harder to fix that then tweaking the rendering of that

above aside, seems there is a luacheck issue with this pr currently

@OgelGames
Copy link
Contributor

No.

The main reason raw formspecs are not supported, as @wsor4035 mentioned, is to avoid sending bad formspecs to clients. Formspecs are a pain to get right, and the touchscreen is intended to be a wrapper for them, to make them easier to use.

Also, scroll containers and the other unsupported elements were intentionally left out for different reasons; for containers and scroll containers, the reason was to discourage complex formspecs, and use multiple pages or tabs instead.

As for your use case of rendering fractals, supporting raw formspecs does nothing to help performance. What you really need is a way to use minetest.encode_png, but even using the combine texture modifier is much better than using individual images.

@OgelGames OgelGames closed this Jun 26, 2024
@TheEt1234
Copy link
Author

TheEt1234 commented Jun 26, 2024

No.

fair, after i started editing, to add the why section i realised it was stupid, shouldve closed it tbh

As for your use case of rendering fractals, supporting raw formspecs does nothing to help performance. What you really need is a way to use minetest.encode_png, but even using the combine texture modifier is much better than using individual images.

The main problem for rendering fractals is that each digiline_send to the touchscreen needed to go thru each element... in the touchscreen, meaning that it would lag badly And yes, combine can be used, but you still need a lot of images actually no holy crap im so dumb, i could've used combine to its fullest potential

@TheEt1234
Copy link
Author

TheEt1234 commented Jun 26, 2024

2. given formspecs are client side, (citation needed) - with malformed formspecs one could crash clients at one point in the past, much harder to fix that then tweaking the rendering of that

so can texture modifiers actually (https://api.minetest.net/textures/#texture-modifiers), i don't think that is a huge concern because the client has to click on the formspec... for their game to crash...

@BuckarooBanzay BuckarooBanzay added enhancement New feature or request invalid This doesn't seem right labels Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants