magefree / mage

Magic Another Game Engine
http://xmage.today
MIT License
1.82k stars 751 forks source link

Window, panel, and column size and position not persisted in many places (+ other prefs) #4952

Closed GrayedFox closed 6 years ago

GrayedFox commented 6 years ago

There are a number of places where the column and window size and position are not persisted when opening and closing the same panel (and thus are also not persisted after client restarts).

There are also some other persistence areas, i.e. remembering the last deck being edited and loading it by default (if it still exists) when the deck editor is opened, as well as remembering the active filters (colour, set, card types, etc) which I think would be pretty low hanging fruit (as my product owner loves to say) which may be easy to implement.

Namely:

I still think it makes sense to keep some defaults - for example showing the cards in list format - when opening the deck editor. I would like to start contributing to xmage so will open a PR and reference this issue!

Edit: removed preferences panel window position and deck editing panel criteria

JayDi85 commented 6 years ago

Nope. It's not so easy as you talk. Need UI and UX rework before implement "persistence" settings feature (xmage already have some "persistance" code for dialog/windows, but it isn't apply to all objects).

XMage MUST have "reset to default" feature, but it doesn't now (it's the most critical feature from that list). E.g. if new player do something bad then he can't fix it and game is broken for him. As example: "unique" filter cause many bad feedback from users and bug reports: #4612

Moreover, you can't easy saves window positions. XMage use same window for different actions and dialogs (like choice pickup dialog, panels with cards list). Users can "lost" their windows (e.g. it's saved out from the screen last time and he can't see it -- game is broken for that player -- example with "disappeared" window: #4353). XMage must catch that and reset windows automaticity (if not then games will be broken for users). Panel example: after match with sideboarding deck editor window "saved" panel position and users can't see cards list in deck editor at all -- they need to reset it by drag splitter.

That's why:

GrayedFox commented 6 years ago

I wasn't aware that xmage was sharing panels/windows for certain user input - thanks for pointing that out.

I am aware that there is a level of persistence already and was looking specifically at preferences file and the ToDo note here - I think it makes sense to implement the user features we have described here (especially the reset filters settings and fail safe sanity checks for ensuring the window is still visible, i.e. if a user plays xmage on a laptop plugged into an external monitor and then unplugs the monitor, we should make sure thing's aren't over sized and are in fact visible).

I think some of your points are a little bit outside of the scope of what I'd like to add here - i.e. auto size options for panels with cards - I really am referring to the active filters inside the deck editor (which doesn't actually have anything to do with window size or position), the column position and size inside the waiting for table pre-tournament lobby (could re-use the same code used to save column size/position inside Tables panel, since that is working nicely), and again the column size/position inside the deck editor.

Would you be happy if I removed the preferences panel position criteria and also the window size of the pre-tournament lobby window criteria?

That would mean not touching the window size but allowing the client to save and load column position and size within the existing window default size and position?

GrayedFox commented 6 years ago

An aside: I would be happy to do the refactoring mentioned in the ToDo note by making a preference loading utility function which is only exposed when it is required, thus keeping the user preference saving and loading logic isolated, thus making the code more DRY and removing some technical debt :octocat: :smile:

JayDi85 commented 6 years ago

You can start with MageDialog and MageDialogState -- it's a base class for all xmage windows (except dialog windows like "preferences"). And it's have "session persistence" code (it's save window settings to memory).

JayDi85 commented 6 years ago

active filters inside the deck editor

No needs to save filter settings -- it's a one time user actions, e.g. user setup cards list for current game/deck and use another settings next time.

the column position and size inside the waiting for table pre-tournament lobby (could re-use the same code used to save column size/position inside Tables panel, since that is working nicely), and again the column size/position inside the deck editor.

It's a good thing to implement.

Would you be happy if I removed the preferences panel position criteria and also the window size of the pre-tournament lobby window criteria?

Make "waiting" window is sizeable and customizable is good. "Preferences window" must have constant position (well, I want to place a "global reset button" in preferences dialog -- e.g. users can use it anytime on "bad" feeling about xmage).

GrayedFox commented 6 years ago

Okay, so the revised scope of this feature request would be:

I would argue it would be a nice to have if the deck editor had these same features for column size and position, however you are right about the filters setting - if we persist filters it's important a user can press a button to instantly reset all active filters so they are looking at what is now the "default" view (all filters disabled) so that they can quickly get the old behaviour if required.

This button is a bit different to the "global reset" button you are referring to, but I think also a nice-to-have inside the deck editor.

That's probably best done in another PR.

I'll start working on figuring out how to get the table waiting stuff working and if it looks good, will move onto refactoring and tidying up the general preferences loading/saving stuff later on. Start small, as they say :wink:

JayDi85 commented 6 years ago

You can also try to research and rewrite code for deck editor settings (split it to two different settings/mode: for sideboarding and for main deck editor).

hooptie45 commented 6 years ago

There are a lot of opportunities in the deck editor, so if we're taking that on 👏. I'd love to see these as stretch goals:

The logic behind the current deck card layout system is very ambitious, and very complex. Custom placement of each card, and complex logic to try and pile them in various ways sounds good in theory; but I'm wondering if all this flexibly is actually worth the cost.

From a usability perspective, while building a deck I personally don't see the need for exact card placement. I'm more interested in the basic stats about the deck, like how many creatures, instants, and lands; and their mana costs. I also want to be able to quickly adjust the quantities of each card. I also usually don't care too much about which set the card is from; a sensible default is fine 99% of the time (the only exception for me is basic lands, for which I prefer full art).

So if we focus on simple groupings(by type, sorted by cmc), and use sensible defaults for the set; I think we'll find that the current xmage dck format is not really needed; as one of the simple formats common on mtg sites would be fine. My editor of choice at the moment is mtggoldfish, and I my most common workflow is to build the deck online, then import it into xmage, playtest it and tweak it in xmage, then save it and convert it back to simple text format and the upload it back to mtggoldfish.

So what does a simple deck format look like rendered?

Simple Render via MtgGoldfish

### Preview ![https://user-images.githubusercontent.com/149671/40290833-e51d89e4-5c8e-11e8-94b2-19f5acfc2f98.png](https://user-images.githubusercontent.com/149671/40290833-e51d89e4-5c8e-11e8-94b2-19f5acfc2f98.png) ### Mtggoldfish text format ```text 4 Arcbound Ravager 4 Arcbound Worker 2 Blade of the Bloodchief 4 Blinkmoth Nexus 4 Cranial Plating 4 Darksteel Citadel 1 Forest 4 Hardened Scales 4 Inkmoth Nexus 1 Island 3 Master of Etherium 4 Ornithopter 2 Paradise Mantle 1 Rite of Passage 4 Spire of Industry 4 Springleaf Drum 2 Steel Overseer 4 Vault Skirge 4 Walking Ballista 2 Dispatch 1 Etched Champion 4 Galvanic Blast 2 Grafdigger's Cage 2 Naturalize 2 Thoughtcast 1 Wear // Tear ```

Same deck loaded into xmage

### Initial Import ![https://user-images.githubusercontent.com/149671/40291054-840869d8-5c90-11e8-9a35-421fce8b48f7.png](https://user-images.githubusercontent.com/149671/40291054-840869d8-5c90-11e8-9a35-421fce8b48f7.png) ### After a few minutes of rearranging to be readable ![image](https://user-images.githubusercontent.com/149671/40292081-1dc09f4a-5c97-11e8-95eb-3252faac81e6.png) ### Xmge DECK Format ```text 4 [AER:181] Walking Ballista 4 [AER:184] Spire of Industry 2 [5DN:142] Paradise Mantle 2 [M11:214] Steel Overseer 4 [5DN:113] Cranial Plating 1 [DOM:254] Island 4 [DST:163] Blinkmoth Nexus 3 [ALA:49] Master of Etherium 4 [DST:100] Arcbound Ravager 2 [ZEN:196] Blade of the Bloodchief 4 [M15:242] Darksteel Citadel 4 [KTK:133] Hardened Scales 4 [DST:104] Arcbound Worker 4 [NPH:76] Vault Skirge 1 [DOM:266] Forest 4 [MBS:145] Inkmoth Nexus 4 [AER:167] Ornithopter 1 [5DN:91] Rite of Passage 4 [BNG:162] Springleaf Drum SB: 1 [SOM:154] Etched Champion SB: 2 [DKA:149] Grafdigger's Cage SB: 2 [NPH:7] Dispatch SB: 2 [RIX:139] Naturalize SB: 1 [DGM:135] Wear // Tear SB: 2 [MRD:54] Thoughtcast SB: 4 [SOM:91] Galvanic Blast LAYOUT MAIN:(3,8)(CMC,true,1)|([AER:167],[AER:167],[AER:167],[AER:167])([AER:181],[AER:181],[AER:181],[AER:181])([DST:104],[DST:104],[DST:104],[DST:104])([DST:100],[DST:100],[DST:100],[DST:100])([ALA:49],[ALA:49],[ALA:49])([KTK:133],[KTK:133],[KTK:133],[KTK:133])([NPH:76],[NPH:76],[NPH:76],[NPH:76])([M11:214],[M11:214])([5DN:142],[5DN:142])([BNG:162],[BNG:162],[BNG:162],[BNG:162])([5DN:113],[5DN:113],[5DN:113],[5DN:113])([ZEN:196],[ZEN:196])([5DN:91])()()()([AER:184],[AER:184],[AER:184],[AER:184])([M15:242],[M15:242],[M15:242],[M15:242])([DST:163],[DST:163],[DST:163],[DST:163])([MBS:145],[MBS:145],[MBS:145],[MBS:145],[DOM:254])([DOM:266])()()() LAYOUT SIDEBOARD:(3,3)(NONE,false,50)|([NPH:7],[NPH:7])([SOM:91],[SOM:91],[SOM:91],[SOM:91])([MRD:54],[MRD:54])([DGM:135])([DKA:149],[DKA:149])()([RIX:139],[RIX:139])([SOM:154])() ```

Comparing a website's display to a native client is comparing apples to oranges, but the logic still works. There are a couple java cleint deck editors which use the paradigm I'm advocating for:

Similar to xmage with piled cards, but the sections and placement are derived

image

A table for the actual deck. Might be usefule, or simple to implement.

screenshot 2018-05-21 01 26 37

The client referenced above renders both ways, based on a UI toggle switch; so you can flip quickly between the different views depending on what you're trying to analyze. This client is also notable because is supports bulk downloading of netdecks; which you can then use or modify for yourself. Great for playtesting against strong decks in your formats metagame.

Very simplistic Table UI, but interesting deck stats (If you know how to read it, which can use some work)

image

This client is very basic, but it saves deck stats and play history per deck.

In summary, we have a usability problem that is rooted in the complex layout logic; which requires the more complex dck format to function; and ultimately leads to contrived workflows with trying to playtest netdecks. If we focus on the user, and what they're trying to do; we might be able to really turn the corner on this. In the process this would also:

Finally, just want to acknowledge the other OSS mtg clients, which are both very good. We're lightyears ahead of them with our server implementation and depth; but they outshine us in other aspects.

It's fine if they wanna do it themselves, but we should be interested in there discoveries; and try to leverage that whenever possible.

GrayedFox commented 6 years ago

Excellent write up man! I'm learning java now (come from a C# and javascript background) so I'll be slow but after poking around the GUI and seeing the amount of times we are repeating our self (code doing similar work) and the complexity of that system a total overhaul of both the GUI and the deck editor I think makes sense - but definitely not in the same PR.

As a start I think I'll use this issue as a bouncing board to get used to the current GUI and start refactoring what I can, but I'm 100% behind you in terms of support and helping to implement/simplify the deck editor too and allowing for easy import/export to a text format