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

Formspec: allow lists to change size and existence while the formspec is open #9700

Merged
merged 1 commit into from
Apr 18, 2020

Conversation

Desour
Copy link
Member

@Desour Desour commented Apr 17, 2020

  • Goal of the PR: Fix a regression.
  • How does the PR work? (All functional are changes listed here to make reviewing easier.)
    • GUIFormSpecMenu::parseList no longer checks for the existence of the inventory.
    • GUIInventoryList::m_geom is now exactly the geom given in the formspec string. If only a smaller inventory should be drawn, that is decided in GUIFormSpecMenu::draw. (Inventories can change their size any time.) Hence there's no more trimming of geom.
    • To avoid a flood of warnings, there's now GUIInventoryList::m_already_warned. Only one warning is written in a row.
    • GUIInventoryList::getItemIndexAtPos is improved. It now also returns -1 if there can not be an item at the specified position because the inventory list is too small.
  • Fixes Inventory not being rendered #9640.

To do

This PR is a Ready for Review.

How to test

@SmallJoker
Copy link
Member

Took the code from #9640 (comment)
After adding minetest.after(0, ...) the error still did not show up: (formspec packet was faster)

finished.
Client: Detached inventory update: "fs_test_detached_inv", mode=update
Client: Detached inventory update: "fs_test_detached_inv", mode=update
Game::handleClientEvent_ShowFormSpec

A delay of >1 server step finally let the warning appear:

finished.
Game::handleClientEvent_ShowFormSpec
2020-04-17 19:23:36: WARNING[Main]: GUIFormSpecMenu::parseList(): The inventory location "detached:fs_test_detached_inv" doesn't exist                                                                                                              
Client: Detached inventory update: "fs_test_detached_inv", mode=update
Client: Detached inventory update: "fs_test_detached_inv", mode=update

PR works as expected. The inventory list shows up after the formspec is already rendered.

@sfan5 sfan5 merged commit 4fb6b6a into minetest:master Apr 18, 2020
@Desour Desour deleted the fs_fix_list_not_there branch April 18, 2020 15:29
nerzhul pushed a commit to nerzhul/minetest that referenced this pull request Apr 27, 2020
Desour added a commit to Desour/minetest that referenced this pull request Jan 20, 2022
* general information
* what happens when inv changes afterwards (see also minetest#9700)
* `W` and `H` are not like for other elements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inventory not being rendered
3 participants