Crafting Dialog not Opening #134

Activity

1. vilarion

   added

   bug

   on Jul 10, 2022
2. vilarion commented on Jul 10, 2022

   

   vilarion

   on Jul 10, 2022

   MemberAuthor

   More actions

   As far as I can tell this only happens when you actually walk up to a tool and use it. Just standing in front of the tool and using it does not seem to cause this issue. Maybe the character is still moving internally, which is why the dialog is not shown.
3. brightrim commented on Jul 10, 2022

   

   brightrim

   on Jul 10, 2022

   Last edited by brightrim

   Member

   More actions

   This also happens in housing when using the construction trowel, where the crafting dialogue is accessed after standing still for a while due to navigating through a selection dialogue first. Housing is not available in the mentioned versions above, but likely will be on live by the time this issue is looked at.
4. Asyxian commented on Sep 24, 2026

   

   Asyxian

   on Sep 24, 2026

   Last edited by Asyxian

   More actions

   I found two client-side issues that can explain this behaviour. The server version mentioned in the report also confirms that dialog IDs can be reused after movement closes a crafting dialog.

   1. Server-side closing leaves the dialog marked as open

   DialogHandler.closeDialogImpl() calls craftingDialog.closeWindow() but does not reset openCraftDialog.
   When another crafting request arrives with the same ID, showCraftingDialogImpl() takes the “same ID and already open” branch. That branch updates the item list without calling show(), so the window remains hidden.
   The flag is reset by closeCraftingDialog(), which handles DialogCraftingCloseEvent. However, the server-close path does not generate that event. Nifty’s WindowControl.closeWindow() emits the generic WindowClosedEvent instead.
   This gives a possible sequence:

   1. Crafting dialog 0 is opened.
   2. Movement causes the server to close it. The client hides the window but retains openCraftDialog = true.
   3. A subsequent crafting request receives ID 0 again.
   4. The client updates the hidden window instead of reopening it.
      In the reported server revision, CraftingDialog::closeOnMove() returns true. Player::closeDialogsOnMove() sends the close command and removes the dialog. requestDialog() can then reuse the freed ID.
      The stale flag persists while standing still, so this mechanism does not require the character to still be moving when the next dialog arrives.

   2. Separate queues can reverse close/open ordering

   showCraftingDialog() queues work in UpdateTaskManager, whereas closeDialog() uses a separate closers queue.
   PlayingState.update() processes UpdateTaskManager before updating the GUI handlers, where the close queue is drained.
   Consequently, if Close(0) followed by Open(0) arrives before the same frame, the client can execute Open → Close, immediately hiding the newly opened dialog.

   Validation and possible changes

   The supplied packet passes length and checksum validation and decodes completely with the current client. It contains a crafting dialog with request ID 0.
   Isolated tests using the unchanged dialog handler and actual Nifty show/hide behaviour reproduce both failures. Initial opening and reopening with a different ID succeed. This establishes the client defects, although the preceding packets needed to identify the exact original sequence are not included in the report.

   Possible fixes:

   • Reset the crafting dialog’s open state consistently in every applicable close path, including server-side closing and cleanup.
   • Treat a same-ID request as an in-place update only when the dialog is genuinely open. Handle reopening during a pending hide effect as well.
   • Process dialog opening and closing through one ordered GUI-thread queue.
   • Add regression tests for server-close/reopen with the same ID, close/open within one frame, and updates to an already open dialog.

   A server-requested close should reset client state without sending an unnecessary close command back to the server.
5. Asyxian

   added a commit that references this issue on Oct 8, 2026

   Merge upstream Illarion-eV#134 into fork develop

   73dd303