Fix deck editor tabs vanishing, deck name bleed, and the commander tab crashFix/deck editor state - #11896
Open
leriomaggio wants to merge 11 commits into
Open
Fix deck editor tabs vanishing, deck name bleed, and the commander tab crashFix/deck editor state#11896leriomaggio wants to merge 11 commits into
leriomaggio wants to merge 11 commits into
Conversation
Limited and quest editors take the deck list, deck generator and probabilities tabs out of the layout while they are open. Each controller kept its own DragCell fields to put them back, but that record is per-controller while the layout is global, so the tabs could be lost permanently: - update() runs whenever an editor becomes current, and a second editor can be constructed for a screen before the first has restored. The second removeTab found the docs already gone and cached null, so resetUIChanges re-added nothing. - removeTab tears down the parent cell when the removal empties it, so the cached cell may no longer be in FView. Re-adding to it put the tab in a cell nothing draws and SLayoutIO does not save. SHiddenTabs now keeps that record for every editor. Hiding an already hidden doc does nothing, and when the original cell has been torn down, restore picks whichever cell grew to cover the space it occupied. It lives in forge.gui.framework alongside DragCell and SLayoutIO, because it is layout state rather than deck editor state. The ten live controllers lose their DragCell fields. removeTab is deprecated; only the unreachable CEditorCommander still calls it (dropped in Card-Forge#11885).
Editors take some tabs out of the layout while they are open. Any layout save during that window wrote them out of the file permanently, because save serialises the live cells and the hidden docs are not in them. The save is easy to trigger without meaning to. SRearrangingUtil.endRearrange runs on a plain tab click, with no drag, and saves whenever the click changed which tab is selected. Tests: four writes in twenty seconds while clicking between the Deck, Statistics and Probabilities tabs during a draft, each one recording the selection change and each one dropping the six hidden docs. SResizingUtil.endResize saves unconditionally on any release over a cell's 5px border strip, so a stray click in a gutter does it too. save() now asks SHiddenTabs which docs are hidden and writes each into the cell it will be restored to, so the file describes where tabs belong.
VCurrentDeck is one panel shared by every editor, and DeckController's updateCaptions is the only thing that writes its title box. CEditorConstructed ends update() with refreshModel, which reaches updateCaptions; CEditorLimited did not, so switching into the draft or sealed editor left the previous editor's deck name in the box. SEditorIO.saveDeck takes the name to save under straight from that box, and CEditorLimited disables it, so the wrong name could not even be corrected before saving. Reproduced by opening the draft deck editor, switching to the Deck Editor and switching back.
Every deck list doc calls VAllDecks.editPreferredDeck from populate(), which routes through DeckManager.editDeck to DECK_EDITOR_CONSTRUCTED. On any other screen that switches screens from inside populate(), while DragCell.setSelected is still iterating that cell's docs: loadLayout clears the list underneath it and the iteration throws ConcurrentModificationException. Reproduced by opening the Token Previewer and clicking its Commander tab, which threw and then dumped the user on the Deck Editor. VAllDecks already guarded its own call for this reason. The guard now lives in editPreferredDeck itself, so the commander, oathbreaker, brawl and tiny leaders lists are covered too, and the caller-side check is no longer needed.
CEditorQuest, CEditorQuestCardShop, CEditorQuestLimited, CEditorTokenViewer, CEditorVariant and CEditorWinstonProcess hid only the constructed deck list and the deck generator, leaving Commander, Oathbreaker, Brawl and Tiny Leaders on screen. The four limited editors have always hidden all six. It was invisible while layouts were losing the docs anyway; now that they survive a save, a Commander Decks tab shows up in places like the Token Previewer and the Spell Shop. This commit fixes that.
leriomaggio
added
GUI
Deck Editor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three related defects in the desktop deck editor. They share a
root cause: state that belongs to the layout or to one editor is held in objects
shared by every editor, with no handshake about owners of the current state.
Fixes #11892
Fixes #11893
Fixes #11894
#11892, tabs vanish permanently
Limited and quest editors take the deck lists, Deck Gen and Probabilities out of
the layout while they are open. Any layout save during that window wrote their
absence to
editor.xml, and every laterloadLayoutthen produced a Deck Editorwithout them.
Two commits:
5730c22moves the bookkeeping out of the controllers intoSHiddenTabs. Tencontrollers each kept their own
DragCellfields, which cannot work when twoeditors are constructed for the same screen and the one that restores is not the
one that hid.
hideis now idempotent, and because it records each cell'sbounds, a cell torn down by the hide is restored into whichever cell grew to
cover that space. Net 214 lines removed.
ACEditorBase.removeTabis deprecated,and only the unreachable
CEditorCommanderstill calls it. This method can be removed as soon as Name the Home and lobby axes "Play Mode" and "Game Format", consolidate the deck editor screens #11885 is merged.c21ce79is the actual fix.SLayoutIO.savenow asksSHiddenTabswhat ishidden and writes each doc into the cell it will be restored to, so the file
describes where tabs belong rather than where they momentarily are not.
SHiddenTabslives inforge.gui.frameworkrather than under the deck editor,since it is layout state and
SLayoutIOneeds it, with no direct dependency withforge.screens.deckeditor.#11893, deck name bleed
VCurrentDeckis one panel shared by every editor, andDeckController.updateCaptionsis the only thing that writes its title box.CEditorConstructed.update()reaches it viarefreshModel(),CEditorLimiteddid not, so the draft and sealed editors showed the previous editor's deck name.
SEditorIO.saveDecktakes the name to save under from that box, andCEditorLimiteddisables it, so the wrong name could not be corrected first.4d8e096adds the missing call. OnlyCEditorLimitedneeds it:CEditorQuestCardShop,CEditorTokenViewerandCEditorNetworkDraftreturn nullfrom
getDeckController().#11894, commander tab crash
DragCell.setSelectedcallsdoc.populate()inside its loop overallDocs.VCommanderDecks.populate()reachesensureScreenActive(DECK_EDITOR_CONSTRUCTED),which on any other screen really switches, so
loadLayoutclears the list beingiterated and the iteration throws.
ef3cf1ffixes the crash.VAllDecks.populatealready guarded its own call forthis reason; the guard now lives inside the shared
editPreferredDeck, so allfive deck list docs are covered by one check instead of five copies.
02154048closes the visibility gap behind it.CEditorQuest,CEditorQuestCardShop,CEditorQuestLimited,CEditorTokenViewer,CEditorVariantandCEditorWinstonProcesshid onlyVAllDecksandVDeckgen, leaving the four commander lists on screen. The limited editors havealways hidden all six. The gap dates from 29263d2.
Note for anyone already affected
This stops the loss but cannot undo it. A layout already saved without the tabs
needs Preferences, Reset Deck Editor Layout, once.