myk's code contributions

[myk]

@goonjamin: I see what you mean -- sync back to r20415 to get it working until I can get Max's changes sorted out.

[Xitax]

Speaking of those, those booster pic files that ended up in /pics_product were uploaded by me, and I thought they were a bit better than the originals that came with the distribution. It took quite a while to find/edit those pictures. If you'd like to use those pictures permanently I'd be gratified.

Attached is my current set of booster pics.

[myk]

We do actually use the .png versions - the .jpg files are no longer downloaded for the booster pics. The ones in the zip you just uploaded are different files than what was in the pics_product/boosters directory, though. Are they new and improved versions?

[Diogenes]

Hey Myk, awesome work! The file migration worked flawlessly, although I do have some feedback based on my own experience:

It would be nice if there were still the option to keep everything within the Forge directory without having to edit a preferences file. I generally prefer not to use my user folder, for general reasons of tidiness and portability. Before the new content management system it was possible to copy my entire Forge folder to a different machine and have it immediately work - I'm unsure whether it's possible to assign the different resource directories within the preference file in an abbreviated form such as "\res\pics" or "\res\decks\constructed" with "C:\...\Forge\" implied, or if it needs a full path starting from the drive letter (which would not be portable), and I didn't tempt fate by trying without asking.

My image directories include complete sets in anticipation of Forge support. As it is, the content importer only grabs the images which are currently downloadable. It would be nice if there were a check-box for users with similar setups to my own that allowed them to simply transfer the entire contents of their set folders (including subdirectories - I use them in my tokens folder) to their new locations, whether Forge recognizes each individual file or not (the different set naming scheme also threw me off and it took me about half an hour to sort out the remaining images by hand - will this be reverted before the next numbered release?)

I had several different sets of decks (sorted by format and theme) located in various subfolders in "\res\decks\constructed\", and the migration grabbed all of the files and placed them in my new "\Roaming\Forge\decks\constructed\" folder. Fortunately I backed everything up beforehand, but I can imagine this causing some frustration if a user has thousands of decks organized in this manner.

Lastly, it would be nice if there were the ability to re-import everything back into the Forge directory. Definitely not a priority, but I can imagine someone using the now default decentralized file system wanting to copy their personalized build to a different machine and have it run on the fly.

Sorry if that sounds like a bunch of complaints! I'm actually really impressed, things went off as intended without a single hitch on the first try of the first build that includes this feature... which, like, never happens. ^_^ Good work!

[Max mtg]

@Diogenes, you may use relative paths in properties file (and keep cache and user data in forge subfolders) to make your installation portable.

@All: I don't know how to detect which pictures are present in a given forge installation.

[myk]

@Max: Could you ensure your new key derivation scheme works in all the following cases:

• quest pet tokens
• Batterskull enters battlefield, creates 0/0 germ token
• non-token card is copied into a token via Heat Shimmer or Cackling Counterpart
• token card is copied into a token via populate (e.g. Druid's Deliverance)

There also seems to be an issue with the downloader now. It's downloading many duplicate files for the LQ pics with artificially high art idices. It should just be downloading as many versions as are listed inside the cardsfolder files.

[myk]

@All: I don't know how to detect which pictures are present in a given forge installation.

what do you mean? for use by which data consumer?

[Max mtg]

Updated some files by r20427 - all points work for me.

About that issue with the downloader - r20424 addresses it.


There one thing remaining - we don't know if the player uses per-set images or pictures dumped into a single folder. It's important to know that in forge.ImageCache.getImageKey(InventoryItem, boolean) because the maximum number of pictures for cards differs for each storage.

Consider HomeLands Abbey Matron(artIndex = 1). It will get an imageKey of HL/Abbey Matron2.full.jpg. Player does keep all pictures in a single folder. He has a single copy of Abbey Matron, named Abbey Matron.full.jpg. So both attepmts to fetch the card - with and without set will fail.

Was this unsolved, or did I break the mechanism that supported correct card image retrieval in that scenario?

[Max mtg]

@All: I don't know how to detect which pictures are present in a given forge installation.

what do you mean? for use by which data consumer?

Would love to know if player has per-set images or all dumped in a single folder. To know before ImageLoader tries to guess which file is present.

[Xitax]

We do actually use the .png versions - the .jpg files are no longer downloaded for the booster pics. The ones in the zip you just uploaded are different files than what was in the pics_product/boosters directory, though. Are they new and improved versions?

Yes, they are.

[myk]

Updated some files by r20427 - all points work for me.

About that issue with the downloader - r20424 addresses it.

awesome. thx! Edit: it looks like the germ token added by Batterskull is still not appearing correctly.

Was this unsolved, or did I break the mechanism that supported correct card image retrieval in that scenario?

This was previously handled by code we discussed earlier

[Max mtg]

So my r20429 restores the LQ setless picture naming rule:
a '1' is appended to cardname if that card exists in multiple printings in any set.

[myk]

That reintroduces a different bug, though. That will prevent the different art indices from ever showing. Ideally, we would map between the requested index and an available index (for example, via if art idx not found, art idx = requested art idx % num available). The code I had was a compromise: use the art index if it exists. If not, choose '1'. Edit: I just looked at the change. It is correct -- it restores the old functionality for lookup. However, the correct number of LQ pictures are not downloaded for the lands anymore.

[Max mtg]

Aha, the number of pictures recognition was incorrect. I got confused by quadruple backslash in String.split call - that turned out to be search for a single slash in the line.
r20434

[myk]

@Diogenes, you bring up some good points, and this might become a long post as I discuss them : p. The big question is, are you the common case or the special case? I have to admit, I made a good number of assumptions when I tried to define the common cases. This is what I came up with:

• Most people use the content downloaders inside forge to get card pics
• There is a secondary, but still large, group of people who download the HQ pics via bittorrent
• Most people update by downloading a release or snapshot and install it into a new directory (as per the update instructions)
• Most people would rather have a hassle-free upgrade process than have to move/import their data every time they upgrade

With this in mind, I made the following decisions:

• Name and organize the pics according to how they are named in the HQ download (which is the same as that used by Magic Workstation). This will let users share the same picture archive with multiple programs and save them hard drive space. It will also let them use the HQ pics without making them rename directories (though the importer could always do that for them if it were necessary)
• Move all user-written and downloadable data out of the forge program directory and into a standard external directory so that upgrades can find the new data immediately without requiring a manual import step.
• Still make it possible for Forge to still be self-contained for the users (like you) that want it that way. Since I assumed this would be the minority of users, I didn't make the configuration for this GUI-based since every option added to the GUI increases its complexity, especially "specialized" options like this. If users who don't know what the implications for upgrade complexity are use this option, it increases the time we have to spend explaining things on the forums.

It could definitely be that some of these assumptions and decisions are incorrect, and if the community tells me so, I am obliged to listen. I do have a question, though:

Is it common to have a directory hierarchy for decks? What is the reason the decks are organized this way? Can the folder hierarchy be replaced with the ability to add tag metadata to decks and modifications to the UI to make decks easier to search through? (The infrastructure for deck tagging has already been implemented by Max, and deck searching is high on my priority list.)

Before the new content management system it was possible to copy my entire Forge folder to a different machine and have it immediately work

As Max mentioned, you can specify relative directories in the forge.profile.preferences file. There is more information and a list of examples in there. If you've already imported your data to the default directory, you can "import" it from there right back to your new profile directory after you set the paths in the preferences file. Just choose the old default directories as your import source.

My image directories include complete sets in anticipation of Forge support. As it is, the content importer only grabs the images which are currently downloadable. It would be nice if there were a check-box for users with similar setups to my own that allowed them to simply transfer the entire contents of their set folders (including subdirectories - I use them in my tokens folder) to their new locations, whether Forge recognizes each individual file or not

When implementing the importer, I debated this behavior with myself. On one hand, there is the danger of importing garbage that will just take up space. On the other hand, that data might prospective, as yours appears to be. I finally decided that the danger of confusing users by automatically bringing in unrecognized data was a greater risk. However, we can add a list of remaining cards that is read by the importer so that the cards are recognized and imported. We can trim this list every once in a while as cards are added for real. There would be no real danger if it gets out of sync, though. It wouldn't matter to the importer whether the cards were matched from the CardDb or the supplementary list. The list could be autogenerated periodically by one of the mtg-data processing tools we have. Edit: see Max's suggestion.

Tokens I'm not entirely sure about. Our naming scheme for set-specific tokens is not exactly consistent. It would need some discussion and research before a proposal could be made there.

(the different set naming scheme also threw me off and it took me about half an hour to sort out the remaining images by hand - will this be reverted before the next numbered release?)

As the whole reason for the new naming scheme is so that we are compatible with MWS and the HQ pics torrent, it is unlikely that the change will be reverted (unless we decide, as a community, that this is a bad idea). Max did just add a capability to map set subdirectory codes, though. Check out the last property in the forge.profile.properties.example file.

Sorry if that sounds like a bunch of complaints!

Not at all, it opens up some great points of discussion! I was hoping to get this stuff out in the open before I merged the branch into trunk, but I do understand that not everyone has time to test experimental branches : )