myk's code contributions
Post MTG Forge Related Programming Questions Here
Moderators: timmermac, Blacksmith, KrazyTheFox, Agetian, friarsol, CCGHQ Admins
Re: myk's code contributions
by squee1968 » 05 Mar 2013, 08:01
Could the pics_product folder be included? Also, could there be a folder that's an extension of the cardsfolder.zip that users could put custom card script files into, and have them read into the program (i.e. half-implemented cards)? Is this plan a prelude to having Forge have the ability to update itself?
Re: myk's code contributions
by myk » 05 Mar 2013, 09:16
good ideas -- once the groundwork is laid, stuff like that should be very easy to add. in-progress cards, overrides, etc. autoupdating, I think, will take a bit of discussion, but this certainly is a first step.
- myk
- Posts: 439
- Joined: 17 Jan 2013, 02:39
- Location: California
- Has thanked: 38 times
- Been thanked: 57 times
Re: myk's code contributions
by myk » 09 Mar 2013, 21:12
Let me write an update on what I've been doing in the myk-separate-writable-data branch:
So far, This is what I've gotten done:
All file paths are now "constified" and listed in NewConstants.java. There is a new class FileLocation that has fields for a default file and a user-specific file. For example:
It was also on my list to remove the old, largely unused properties and localization framework. Once the file constants were moved out (necessary since they now depend on variable profile root dirs), and a few string constants were moved into the code, there was really nothing left. I was able to remove the following files:
My next tasks in this branch are to write the migration code that will move user files to their new locations and to test, test, test to ensure I didn't break anything.
Other changes:
So far, This is what I've gotten done:
- moved all writable files out of the program directory and into (configurable) profile directories
- removed 20MB of images under res/pics_product and arranged for them to be downloaded with the other quest-related content (thanks, Marc, for getting the files straightened out on the server!)
- aligned the directory structure and file names of the downloaded card images to what is in the HQ pics torrent
- refactored the code so that filenames are created in exactly one place instead of copy-pasted throughout the codebase
- cleaned out a bunch of unused cruft from the src and res trees
- Windows
- userDir=%APPDATA%/Forge
- cacheDir=%LOCALAPPDATA%/Forge/Cache (or %APPDATA%/Forge/Cache for windows versions before the local/roaming directory split)
- OSX
- userDir=$HOME/Library/Application Support/Forge
- cacheDir=$HOME/Library/Caches/Forge
- Linux
- userDir=$HOME/.forge
- cacheDir=$HOME/.cache/forge
All file paths are now "constified" and listed in NewConstants.java. There is a new class FileLocation that has fields for a default file and a user-specific file. For example:
- Code: Select all
public static final FileLocation EDITOR_PREFERENCES_FILE = new FileLocation(_DEFAULTS_DIR, _USER_PREFS_DIR, "editor.preferences");
It was also on my list to remove the old, largely unused properties and localization framework. Once the file constants were moved out (necessary since they now depend on variable profile root dirs), and a few string constants were moved into the code, there was really nothing left. I was able to remove the following files:
- src/main/java/forge/properties/ForgeProps.java
- src/main/java/forge/util/TreeProperties.java
- res/**/*.properties (howto file was updated and stored in a plain text file)
My next tasks in this branch are to write the migration code that will move user files to their new locations and to test, test, test to ensure I didn't break anything.
Other changes:
- removed file comments I came across that provided no information, such as field comments of the form "/** */" or function comments that just mention the function name and the types of the parameters. These kinds of comments are obviously autogenerated and are useful when they have additional information, but just take up space when left unfilled.
- moved CardUtil.buildFilename() functionality to CardPrinted. Filenames returned are now always expected filenames. Now that all images have the same name format, the on-disk file system no longer needs to be searched when creating the expected filename for a particular card face. Expected filenames are extensionless as well, so .jpg or .png files can be used interchangeably (such as what is already done with booster pack images).
- ImageCache is now solely responsible for filesystem access (including file existence testing) when loading images.
- card backs are now properly shown for morphed cards
- removed unused CardRankings class and related logic
- removed some svn:ignore metadata now that non-program files will no longer be created in those directories
- refactored common logic in ForgePreferences and QuestPreferences into a base class
- cleared out unused constants from NewConstants.java
- myk
- Posts: 439
- Joined: 17 Jan 2013, 02:39
- Location: California
- Has thanked: 38 times
- Been thanked: 57 times
Re: myk's code contributions
by Max mtg » 09 Mar 2013, 21:30
I have never seen cases, when this code would come handy
- Code: Select all
// try lowering the art index to the minimum for regular cards
if (null == ret && null != setlessFilename && setlessFilename.contains(".full")) {
ret = _findFile(key, path, setlessFilename.replaceAll("[0-9]*[.]full", "1.full"));
}
Single class for single responsibility.
- Max mtg
- Programmer
- Posts: 1997
- Joined: 02 Jul 2011, 14:26
- Has thanked: 173 times
- Been thanked: 334 times
Re: myk's code contributions
by Max mtg » 09 Mar 2013, 21:33
please remove all that getImageLocator code from cardPrinted. This is not related to card - that's why rules how to build a filename should be kept in some other class.
I would even remove getImageFilename from all InventoryItem descendants. It's not their responsibility to report how to display them.
I would even remove getImageFilename from all InventoryItem descendants. It's not their responsibility to report how to display them.
Single class for single responsibility.
- Max mtg
- Programmer
- Posts: 1997
- Joined: 02 Jul 2011, 14:26
- Has thanked: 173 times
- Been thanked: 334 times
Re: myk's code contributions
by myk » 09 Mar 2013, 22:02
Some context for everyone else who isn't familiar with this code path:Max mtg wrote:I have never seen cases, when this code would come handyWould you please describe when it's needed?
- Code: Select all
// try lowering the art index to the minimum for regular cards
if (null == ret && null != setlessFilename && setlessFilename.contains(".full")) {
ret = _findFile(key, path, setlessFilename.replaceAll("[0-9]*[.]full", "1.full"));
}
This code appears in the fall-back algorithm where the ImageLoader attempts to find an image file to load. First it tries the full path (e.g. <card pics dir>/GTC/Card Name8.full.jpg). Then it tries it without the set specification (e.g. <card pics dir>/Card Name8.full.jpg). Finally, it tries it with a minimum art index (e.g. <card pics dir>/Card Name1.full.jpg) -- that's the step shown in the code snippet above. Of course, if the card only had one art index to begin with, the appended number does not appear and the last fallback step is not attempted.
The case that this is meant to address is where LQ pics have been downloaded, but LQ set pics have not, and the LQ set pics have a higher index than the non-set LQ pics. Non-set LQ pics come from a different URL list, and the art indices don't always match up (note, however, that even if the non-set LQ pics only have one URL, the file is named with a trailing '1' if there is more than one art index globally). This code comes into play mostly with the basic lands. Plains, for example, has this in cardsfolder/p/plains.txt:
- Code: Select all
SVar:Picture:http://resources.wizards.com/magic/cards/unh/en-us/card73963.jpg\http://gatherer.wizards.com/handlers/image.ashx?multiverseid=8322&type=card\http://gatherer.wizards.com/handlers/image.ashx?multiverseid=159288&type=card\http://gatherer.wizards.com/handlers/image.ashx?type=card&multiverseid=4428
Oracle:W
SetInfo:8ED Land x4
SetInfo:H09 Land
SetInfo:S99 Land x4
SetInfo:ZEN Land x8
...
This used to work similarly, but a little differently. The downloader used to name the first file of several with the same name as card_file.jpg. The second would be card_file1.jpg, and so on. The fallback algorithm would fall back to card_file.jpg if the index inherited from the set was too large. The change was made because now the files are all named according to the MWS standard.
edit: I forgot to mention, the main reason I moved the fallback code from CardUtil's buildFilename to ImageLoader is that now it only gets called when the file is loaded and won't get called again if the file is cached. Before, we were probing the filesystem on every new card instance instead of just when loading a new image.
I realize that this would be ideal, but the current architecture would require quite a bit more refactoring to be able to build the image paths consistently without building the path via that API. If you have some specific ideas of how to do it, I'm all ears, but the method I tried (reflection in ImageLoader) didn't work out the way I hoped it would. I could certainly consolidate all filename-building logic into a separate class, and have the existing API call that for the actual business logic, if you'd prefer.Max mtg wrote:please remove all that getImageLocator code from cardPrinted. This is not related to card - that's why rules how to build a filename should be kept in some other class.
I would even remove getImageFilename from all InventoryItem descendants. It's not their responsibility to report how to display them.
- myk
- Posts: 439
- Joined: 17 Jan 2013, 02:39
- Location: California
- Has thanked: 38 times
- Been thanked: 57 times
Re: myk's code contributions
by myk » 09 Mar 2013, 22:37
Here is a list of other changed resource files and directories not mentioned in my previous post:
There are also some files that look like they could be moved to the server and dynamically downloaded to reduce our distributable size, but that can be done later:
There are more that look like they could be removed, but I wanted to make sure nobody was using them:
- res/decks/... (cubes moved to res/defaults/cube/ in preparation for user extension)
- res/draft/{common,rare,uncommon}.txt (unused)
- res/lang/... (howto/en.properties reformatted and moved to res/howto.txt)
- res/layouts/... (*_default.xml files moved to res/defaults/)
- res/pics/... (now in user cache dir)
- res/pics_link/... (unused)
- res/pics_product/... (moved to server)
- res/preferences/... (default preferences moved to res/defaults/)
- res/product-images.txt (split up into individual files, one per dir)
- res/quest/all-prices.txt (now downloadable)
- res/quest/price.txt (unused, or at least I couldn't find any reference to it)
There are also some files that look like they could be moved to the server and dynamically downloaded to reduce our distributable size, but that can be done later:
- res/draft/rankings.txt
- non-default skins
- sounds
There are more that look like they could be removed, but I wanted to make sure nobody was using them:
- res/AllTokens.txt
- res/gamedata/
- res/cardsfolder/mkzip.sh (duplicate of pom.xml functionality?)
- myk
- Posts: 439
- Joined: 17 Jan 2013, 02:39
- Location: California
- Has thanked: 38 times
- Been thanked: 57 times
Re: myk's code contributions
by timmermac » 10 Mar 2013, 00:59
OK, so my question here would be: Where do those of us that have used forge for many, many moons put our card pics that we're accustomed to putting in /res/pics now? I'd rather have the /res/pics folder included with the distribution for that. Also, isn't the /res/pic_products folder supposed to be where the booster, starter and other images of that type go?myk wrote:Here is a list of other changed resource files and directories not mentioned in my previous post:
- res/decks/... (cubes moved to res/defaults/cube/ in preparation for user extension)
- res/draft/{common,rare,uncommon}.txt (unused)
- res/lang/... (howto/en.properties reformatted and moved to res/howto.txt)
- res/layouts/... (*_default.xml files moved to res/defaults/)
- res/pics/... (now in user cache dir)
- res/pics_link/... (unused)
- res/pics_product/... (moved to server)
- res/preferences/... (default preferences moved to res/defaults/)
- res/product-images.txt (split up into individual files, one per dir)
- res/quest/all-prices.txt (now downloadable)
- res/quest/price.txt (unused, or at least I couldn't find any reference to it)
There are also some files that look like they could be moved to the server and dynamically downloaded to reduce our distributable size, but that can be done later:
- res/draft/rankings.txt
- non-default skins
- sounds
There are more that look like they could be removed, but I wanted to make sure nobody was using them:
- res/AllTokens.txt
- res/gamedata/
- res/cardsfolder/mkzip.sh (duplicate of pom.xml functionality?)
IIRC, /res/quest/prices.txt is supposed to be the baseline for quest spell shop card prices, unless that got moved somewhere else.
"I just woke up, haven't had coffee, let alone a pee in 7 days, and I find out you stole my ass and made a ...mini-me! Carter, I should be irked currently, yes?" - Jack O'Neill
Re: myk's code contributions
by myk » 10 Mar 2013, 01:19
If you want to keep the card pics in the distribution directory, that's absolutely ok. The user profile dir and cache dir are configurable, and you can put them where ever you want. The default paths, which should keep the program directories clean for shared installations and make program upgrades hassle free (e.g. no importing of pics or data required) for users who upgrade by extracting/installing to a new location, are detailed a few posts up.timmermac wrote:OK, so my question here would be: Where do those of us that have used forge for many, many moons put our card pics that we're accustomed to putting in /res/pics now? I'd rather have the /res/pics folder included with the distribution for that.
Most of those files were already on the server and were meant to be downloaded like the other quest data. It just never got hooked up to a download button. I just finished that work here. Those pics will be downloaded into the cache dir along with all the other downloadable data.Also, isn't the /res/pic_products folder supposed to be where the booster, starter and other images of that type go?
Looking at the code, it seems prices come from all-prices.txt now, which gets updated with the downloader (and goes into the cache dir too).IIRC, /res/quest/prices.txt is supposed to be the baseline for quest spell shop card prices, unless that got moved somewhere else.
My working layout for the user and cache directories is:
- Code: Select all
<userDir>/
decks/
constructed/
draft/
...
preferences/
quest/
<cacheDir>/
pics/
cards/
icons/
tokens/
...
db/ (downloaded database files like the quest card prices)
edit: implemented - cardPicsDir is now separately configurable
- myk
- Posts: 439
- Joined: 17 Jan 2013, 02:39
- Location: California
- Has thanked: 38 times
- Been thanked: 57 times
Re: myk's code contributions
by Chris H. » 10 Mar 2013, 02:31
timmermac wrote:OK, so my question here would be: Where do those of us that have used forge for many, many moons put our card pics that we're accustomed to putting in /res/pics now? I'd rather have the /res/pics folder included with the distribution for that. Also, isn't the /res/pic_products folder supposed to be where the booster, starter and other images of that type go?
IIRC, /res/quest/prices.txt is supposed to be the baseline for quest spell shop card prices, unless that got moved somewhere else.
Over the last several years people have had to jump through a lot of hoops to update to the most recent version. We hope to reduce this workload to a point where a new user will find that it is fairly painless to update.

Us long term users will have to adapt and adjust but it will be in our best interests.

The prices.txt file was Rob's idea to provide a list of card prices that would ship with the project. This list was out of date and one of the devs at some point included an updated all-prices.txt file. This all-prices.txt file overrides the original file and as such the original file may serve to cause some confusion with the new members. The
-
Chris H. - Forge Moderator
- Posts: 6320
- Joined: 04 Nov 2008, 12:11
- Location: Mac OS X Yosemite
- Has thanked: 644 times
- Been thanked: 643 times
Re: myk's code contributions
by Xitax » 10 Mar 2013, 03:22
Is there a chance that this will fix the bug of not ever picking [land]1.jpg?
Re: myk's code contributions
by myk » 10 Mar 2013, 04:05
yes, that bug is fixed in the branch. Btw, the branch is fully usable at this point, so feel free to check it out (and report any anomalies you see!). Unless there are further suggestions or concerns, the only work left to do before it is mergeable is the migration code.
- myk
- Posts: 439
- Joined: 17 Jan 2013, 02:39
- Location: California
- Has thanked: 38 times
- Been thanked: 57 times
Re: myk's code contributions
by Xitax » 10 Mar 2013, 04:48
Thanks, myk! Your contributions lately have been impressive.
Re: myk's code contributions
by myk » 14 Mar 2013, 08:37
Hi all!
I'm gearing up to merge in the work I've been doing to separate the program data from user-writable data. I have taken care to make the data migration safe and as painless as possible, but I was hoping to get validation from a few additional users that (1) everything gets migrated correctly and (2) what is happening is clearly communicated. If anyone has a few minutes, I would really appreciate your help! The procedure is safe -- you aren't going to lose any data even if things go horribly wrong. Just follow the following steps:
Once the migration is done, play with Forge a bit as you usually do. Make sure all your data is there, and please report back on your experiences! As I fix any bugs that come up and make further enhancements, I'll upload updated versions and write a notification here, but the link above will always get you to the most recent version.
Thanks!
--Myk
I'm gearing up to merge in the work I've been doing to separate the program data from user-writable data. I have taken care to make the data migration safe and as painless as possible, but I was hoping to get validation from a few additional users that (1) everything gets migrated correctly and (2) what is happening is clearly communicated. If anyone has a few minutes, I would really appreciate your help! The procedure is safe -- you aren't going to lose any data even if things go horribly wrong. Just follow the following steps:
- Copy your entire forge installation to another location on your hard drive. Be sure to copy from the installation root so you include all your picture files, quest data, and decks.
- Download the snapshot build of my branch from http://cardforge.org/releases/snapshots/forge/branches/myk-separate-writable-data/current/
- Install it over the copy that you made
- Run it! The prompts that automatically come up should guide you the rest of the way.
Once the migration is done, play with Forge a bit as you usually do. Make sure all your data is there, and please report back on your experiences! As I fix any bugs that come up and make further enhancements, I'll upload updated versions and write a notification here, but the link above will always get you to the most recent version.
Thanks!
--Myk
- myk
- Posts: 439
- Joined: 17 Jan 2013, 02:39
- Location: California
- Has thanked: 38 times
- Been thanked: 57 times
Re: myk's code contributions
by Agetian » 14 Mar 2013, 09:39
@ myk: I'm glad to hear you're progressing well with your branch so far! I'll see if I can give it a test in the next couple days
I have a question though - I can see that your code restructures the way Forge loads data and, more importantly, where it's loading it from - since I'm working on the AI profiles I'm currently storing them the old-fashioned way in "res/ai", which, I believe, will become an obsolete location after your branch is merged. What should I do with the location of the AI folder and how should I determine it after your branch is merged? Would you be willing to help out a bit with that?
- Agetian

- Agetian
- Agetian
- Programmer
- Posts: 3486
- Joined: 14 Mar 2011, 05:58
- Has thanked: 683 times
- Been thanked: 569 times
Who is online
Users browsing this forum: No registered users and 27 guests