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

Add primitive formspec testing feature #92

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Conversation

y5nw
Copy link
Contributor

@y5nw y5nw commented Jan 12, 2023

This should at least partially address #53.

This PR is still WIP. Specifically, it does not provide an API to "interact" (*) with formspec elements, and formspecs need to be parsed in order to send the fields correctly.

(*) Whether the specific element is present is a different matter. IMO not checking this allows mods to test whether the callback handles certain situations correctly, e.g. formspecs where some parts are conditionally hidden but have only one callback function (which also need to have the code to make sure certain fields are not handled when they are not supposed to be).

@S-S-X
Copy link
Owner

S-S-X commented Jan 11, 2025

I've done some work with formspec testing, unfortunately I've messed up local dev and have not pushed anything here in long time. Trying to get it sorted out... some day.

I'll try to merge this PR with what I've done so far (formspec parsing, looking for elements, player formspec, etc.).

@S-S-X S-S-X added in-latest-docker Included with latest docker images, might not yet be available elsewhere in-dev-testing Some work has been done and results are being tested on dev branch. and removed in-latest-docker Included with latest docker images, might not yet be available elsewhere labels Jan 11, 2025
@y5nw
Copy link
Contributor Author

y5nw commented Jan 11, 2025

Oops, looks like I have forgotten this PR. Admittedly it was too ambitious for my skills at the time so it ran out of steam quite quickly.

@S-S-X
Copy link
Owner

S-S-X commented Jan 12, 2025

Did some rebasing in an attempt to mix in your work and to get somewhat clean history out of it, preview here:
https://github.com/S-S-X/mineunit/compare/v0.13.0...v0.14?expand=1

Howevert, your last commit Add primitive wrapper for quitting formspecs isn't there, others are in one form or another.

Things I should probably drop before making a release:

  • Player._formspec name + content table, while going through rebases it was an attempt to keep also the way you did it but done that I'm pretty sure it should be dropped and just Form class instances should stay.
  • Actual field and action handling form Form instances, my idea was basically this:

    mineunit/formspec.lua

    Lines 262 to 274 in 0db49bb

    -- Get fields that would be submitted with any of the basic submit actions.
    -- TODO: Allow specifying trigger, like button or scrollbar for example, and return fields based on action.
    -- FIXME: Currently results will be wrong: includes everything like buttons that shouldn't be there.
    function Form:fields()
    mineunit:error("Form:fields() is experimental and UNSTABLE. Its behavior and interface WILL BE CHANGED.")
    local results = {}
    for e in ivalues(self._data) do
    if submit_fields[e:type()] and e:name() ~= "" then
    results[e:name()] = e:value()
    end
    end
    return results
    end
  • Maybe clean up tests a bit...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-dev-testing Some work has been done and results are being tested on dev branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants