Forge version 1.5.11

[Chris H.]

Tentative target release date: .

[drdev]

So I just committed a big change that has swapped out all the DeckLister controls for DeckManager controls, which work more like the CardManagers used for deck building. This involved a large refactoring of the table column logic, and for the time being this means you won't be able to save column changes between sessions (I plan to fix that before the release). However, I hope you'll agree the upsides of the new DeckManager UI is worth it, namely the ability to view and sort on the color and formats for each deck (in addition to name, main, and side), as well as search by name and filter by color, format, sets, and quest worlds.

Here's a screenshot of what it looks like:

Let me know what you think and if you encounter any issues with it.

Thanks.
-Dan

[Agetian]

r24230 and onwards: Starting to commit an experimental code change that will eventually lead to full support of distinction between cards with different card indexes everywhere in the game, as well as random generation of cards with different card indexes in card pools such as Sealed Deck, Booster Draft, Quest Spell Shop, and Quest Starting Cardpool.

This change modifies some interactions with the card database, so unwanted side effects and NPEs are theoretically possible. Please report those and assist if possible. Also, some code might be a little quirky/suboptimal in CardDb.tryGetCard, please assist if necessary and possible.

If anybody knows how to best handle random card art index generation for the card pools listed above (and especially if anybody knows where it is done - it seems it's all done in different places of the code), your help is welcome.

EDIT: r24231: Basic lands with different art are now generated in Quest Spell Shop and Quest Starting Cardpool.

EDIT: For some reason, the current code misbehaves in Constructed deck editor. Right now, the card art index is saved into .dck files and is loaded from them, but the deck editor (the Constructed deck editor, specifically) will quietly ignore all the cards that have different card art indexes from the first one. For instance, the following:

Code: Select all

20 Forest|7ED|1
20 Forest|7ED|2
20 Forest|7ED|3


will load only the first Forest entry and show 20 Forest cards with the same card art index. The other Forests will be ignored. :\ What am I missing here?
P.S. Works fine in Sealed and Draft if I specify different art indexes there (they show up just fine).

EDIT: Looks like the above is caused by the fact that the "Show unique cards only" option no longer works (was commented out, not replaced by anything) and that the card catalog only shows one card of each type in Constructed deck editor (not even with a set restriction). Please correct this, showing cards from all sets and with all card indexes is crucial to supporting cards with different art.

EDIT: r24236: Restored the correct Constructed deck editor functionality by temporarily showing all cards (from all card sets and with different card art indexes) at all times. Note that the code that supports the "Show unique cards only" feature is currently broken because it was commented out by someone. When restoring that functionality, recoding it or doing whatever else was meant to be done with it, please note that: a) the deck editor must *always* show all cards (with the set info and the card art index taken into consideration), otherwise some cards will be ignored when displaying/editing Constructed decks; b) if you remove the "Show unique cards only" option altogether, please make sure that you leave the functionality to show all cards (from all sets and with all card art indexes) as default for the card catalog as well because otherwise there's no way to customize decks by including cards from different sets and with different art indexes. So, in CEditorConstructed.java in the constructor:

Code: Select all

CardManager catalogManager = new CardManager(false); // ONLY set this to 'true' if you restore the "Show Unique Cards Only" toggle in the card display options. If you remove the toggle altogether, leave this at false.
CardManager deckManager = new CardManager(false); // LEAVE this at 'false'. Setting it to 'true' will cause confusion with some cards (actually in the deck) not being shown in the deck editor.


EDIT: r24238: Booster Draft and Sealed Deck modes correctly handle and support cards with different art indexes (when generating the random basic lands in the pool, in particular).

EDIT: r24252: Now the game will randomize card art if it's not specified, so "20 Forest|7ED" essentially means "twenty Forests from 7th Edition with random art". Also, e.g. "20 Forest" now means "20 Forests from the latest set with random art". This also means that preconstructed (non-random) decks in Constructed mode will now have randomized card art.

- Agetian

[drdev]

Agetian, I'm the one who commented out the preference code for "Show unique cards only" as a temporary measure during my refactoring of columns for card and deck managers. I plan to add it back in as soon as possible (certainly before the release) as part of a refactoring of the preference saving logic for card and deck managers. The Editor Preferences pane will go away as part of this, with the preferences instead being available on each item manager instances in a panel that will appear if you click an Options button next to the "List View" combo box. This will allow preferences to be different between different editors, and make it much easier to re-use the card and deck managers elsewhere, such as for displaying zones like library and graveyard during game play.

[Diogenes]

Let me know what you think and if you encounter any issues with it.

Thanks.
-Dan

It looks and works great! I'm posting some observations over in the UI discussion thread.

[Agetian]

@ drdev: Gotcha, thanks for telling me! Yes, take your time, it's not a high priority task by any means (in the context of what I'm doing), just keep in mind those gotchas I outlined above when restoring the functionality (basically, the unique-only property shouldn't affect the deck list, only the catalog - the deck should always show all cards, including non-unique ones).

@ Diogenes: Thanks! Alrighty!

- Agetian

[Diogenes]

@ Diogenes: Thanks! Alrighty!

My post in the UI thread was in regards to drdev's changes, but I'm really excited about what you're doing too!

Anyway, I do have one quick question for you. Now that we can specify a unique card ("20 Forest|7ED|1"), what happens if just the set is specified? ("20 Forest|7ED" for instance.) Will the functionality remain as it is now, where each forest will have a random image from 7ED?

It sounds like it will become possible to add "generic island" (if "Show Unique Cards Only" is checked) or "Urza's Saga Island #4" (if it's unchecked) from the deck editor, but not "generic Urza's Saga island", which is probably the most common desired usage (or I might have this entirely wrong in my head.)

Again, thank you for making this happen!

[Max mtg]

Does not the newest format always include the older ones? I.e. if a deck is legal in Modern, it's also legal in Legacy and Vintage.

PS: Core classes are not a suitable place to cache data (like color of deck or its format legality). So I've cleaned up fields and methods in Deck, ManaCost and their neighbours.

[drdev]

Does not the newest format always include the older ones? I.e. if a deck is legal in Modern, it's also legal in Legacy and Vintage.

PS: Core classes are not a suitable place to cache data (like color of deck or its format legality). So I've cleaned up fields and methods in Deck, ManaCost and their neighbours.

It's not GUI data that's being cached though, it's an actual property of the Deck. That's like saying we shouldn't be caching the card list and should look it up from the filesystem each time you want to iterate a deck's cards. I'm really trying to go for good performance here since these properties are looked at frequently by ItemManager code, and I don't understand your reasoning for saying it's not ok. The fact that these properties are currently only accessed from GUI code does not make it GUI code.

To your first question, some cards are legal in Standard but banned in Legacy for example, so it's good to list them all.

EDIT: After looking over what you did, I'm ok with it. I did tweak a couple things which was to make the default format column sort put more recent formats on top (like Standard), and make the color column left aligned inline with a change I was already planning to make (and now have made) to left align the column headers. This way everything is now left aligned.

[Agetian]

@ Diogenes: As of right now, the line "20 Forest|7ED" is essentially the same as "20 Forest|7ED|0", so only one picture (the first one) will be used. I'm thinking of a way to make the game randomize the card art if the art index is not specified - haven't figured out a way yet (which is why for now randomization only works for random-generated decks but not for premade ones), but I'm working on it!

EDIT: r24252: Now the game will randomize card art if it's not specified, so "20 Forest|7ED" essentially means "twenty Forests from 7th Edition with random art". Also, e.g. "20 Forest" now means "20 Forests from the latest set with random art" (I'm still unsure whether it's best to randomize the set for the preconstructed decks as well or not... for now I'm using the default latest set; fully random color decks still take cards from different sets, not only from the latest set.). This change also means that preconstructed (non-random) decks in Constructed mode will now have randomized card art unless a specific card art index is requested in the deck file.

The sad part of this change is that I haven't found a way (at least not yet) to add cards with random art into the deck optimally, so the game has to "reget" a card with tryGetCard every time it wants to insert a card with random art index, and it has to insert them into the deck one by one in a 'for' loop. Batch-inserting them with a specified count is not possible at the moment because otherwise the game randomizes the card art only once, so essentially you get a certain number of cards inserted with the same card art index. If I find a way to make this more optimal (and work at the same time) I'll try to do it more optimally... if anyone knows a better way already, feel free to optimize.

- Agetian

[Max mtg]

@drdev,
You cannot just cache current color, sets legality in a Deck... even if these fields are marked as transient. Because in that case you'll have to track changes to deck's sections (like add card or remove one) and invalidate the cached values. Since Deck will never be notified that someone changed its section, it's very easy to reach a state when wrong data is cached and considered valid.

Still, if you need those values somewhere cached, there could a proxy be used... something like DeckBox class with references to a deck, and cached values and some kind of 'recalculate' method. But again, not in core.

Ok, check against multiple GameFormats is not a problem - already commited.


Your deck list seems to ignore decks found in subfolders, that are supported by the actual FDeckChooser.

[Diogenes]

I hope no-one minds me posting so often today since I don't do any work around here; several topics that resonate with me just happened to come up at the same time. If I'm getting in the way, just let me know and I'll cut back.

Anyway, thanks for the updates Agetian, it sounds like art-management is getting to a really good place!

EDIT: r24252: Now the game will randomize card art if it's not specified, so "20 Forest|7ED" essentially means "twenty Forests from 7th Edition with random art". Also, e.g. "20 Forest" now means "20 Forests from the latest set with random art"

All I can offer is my opinion on how this should work, if that's alright. I like how you're handling "20 Forest|7ED" and I think that's in its perfect place.

For decks with unspecified lands, I think the ideal would be to randomly select one set that has images for all the basics used by the deck. For instance, if the deck is BR, it could pull from the variety of CHK swamps and mountains as if they were all assigned "Land|CHK". The basics are usually thematically-consistent in a set, and this would let users enjoy the manabases of days past, without mixing white and black borders etc (although it might be nice to retain "totally random land selection" as a preference - why not have 37 different plains?)

I've also noticed that recently Forge has stopped selecting lands from sets with a low number of images (this may have to do with the line "SVar:PicCount:4" in the basics' cardfolder data.) If a user has added set info for APL, Euro, Guru, or Un-lands they'll never see them randomly selected. It would be great if this change could be reversed.

(I'm still unsure whether it's best to randomize the set for the preconstructed decks as well or not... for now I'm using the default latest set; fully random color decks still take cards from different sets, not only from the latest set.). This change also means that preconstructed (non-random) decks in Constructed mode will now have randomized card art unless a specific card art index is requested in the deck file.

Would it be possible (with Xitax's permission) to use the code from his automated set-assignment tool? I feel this would be the optimal behavior for unspecified user-created decks (just process them as they're loaded, perhaps with an option to over-write the file,) and while I don't know if the code is compatible it's complete and works well.

Anyway, I've said thanks around 80 times today - it might be getting cliche, but I really do appreciate the work you guys put into this project. Thanks everyone!

EDIT: I just realized that I've proposed two conflicting "best" behaviors regarding random lands (from a random set for all basics / from Xitax's auto-assigner.) XD Let me try that one more time: for all instances where the deck is not set-specified (automatically or otherwise), drawing basics from one set/block would be my default behavior of choice (also, I can be stupid sometimes.)

Extra bit: on the subject of lands, a while ago a user pointed out that the way the set file for heroes vs monsters is written, Forge only recognizes that it has four mountains (instead of eight.) I've attached an adjusted set list that fixes the problem if anyone would like to commit it.

[Max mtg]

Would it be possible (with Xitax's permission) to use the code from his automated set-assignment tool? I feel this would be the optimal behavior for unspecified user-created decks (just process them as they're loaded, perhaps with an option to over-write the file,) and while I don't know if the code is compatible it's complete and works well.

Not sure if that algorithm was Xitax's idea viewtopic.php?f=52&t=6636#p84980 (That is to say no permission is needed)

Should that be implemented in Forge, the Deck Convertor mentioned above becomes obsolette.

[Agetian]

It sounds like a good idea! The said code was inspired by a script I wrote a long time ago, by the way. Once I get everything else art-related working as desired I might try to integrate this functionality into the game, I'll keep you posted.

P.S. Committed the updated set file to SVN, works fine on my end. Thanks!

- Agetian

[Xitax]

Yes you certainly don't need permission but anyway it doesn't matter because I'd be happy to see the functionality get integrated.

It does however need 2 bits of external information, currently supplied by the two external data files. They are not necessarily in optimal format, but it works:
-A hierarchy file to put sets in order by date or by preference.
-A file with each card and all sets it is in.
I imagine both these can be got from Forge's current data files, just stored in a different manner. The idea behind the converter is pretty simple, and the implementation is only dependent on how exactly you want to perform the parsing. I'd assume you'd want to do a rewrite for that reason.

I've spent quite a bit of time editing the "mtgdata.txt" file, gutting all the card info except for set data, editing, fixing bugs. Ideally, a file like this could be automatically created in terms of keeping updated, but I couldn't be bothered so I did it by hand . Like Agetian, I really wanted set assignment to work right because it makes a big difference to me in terms of immersion.