It is currently 22 May 2025, 21:42
   
Text Size

myk's code contributions

Post MTG Forge Related Programming Questions Here

Moderators: timmermac, Blacksmith, KrazyTheFox, Agetian, friarsol, CCGHQ Admins

Re: myk's code contributions

Postby 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?
squee1968
 
Posts: 254
Joined: 18 Nov 2011, 03:28
Has thanked: 110 times
Been thanked: 45 times

Re: myk's code contributions

Postby 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

Postby 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:
  • 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
If the file forge.profile.properties exists in the program directory, the properties userDir and cacheDir can be custom defined. Otherwise, the directories are defined to platform standard locations:

  • 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");
which indicates that the file res/defaults/editor.preferences contains default values and should be loaded first, but user overrides will be loaded from <userDir>/preferences/editor.preferences if that file exists. Currently, the number of FileLocation elements is small, but once this branch is merged, we can start adding other kinds of overrides/overlays, like user-specific worlds or even a cardsfolder overlay.

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)
If we ever want to do localization again, there are several standard frameworks out there we can use.

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

Postby 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"));
        }
Would you please describe when it's needed?
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

Postby 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.
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

Postby myk » 09 Mar 2013, 22:02

Max mtg wrote: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"));
        }
Would you please describe when it's needed?
Some context for everyone else who isn't familiar with this code path:
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
...
You can see that while it only has four "default" LQ URLs defined, some sets have more than 4 lands. If a ZEN Plains card with art index 6 is selected, and the code has to fall back to the non-set LQ pics, reducing the art index to 1 will allow it to successfully find a file.

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.

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.
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.
myk
 
Posts: 439
Joined: 17 Jan 2013, 02:39
Location: California
Has thanked: 38 times
Been thanked: 57 times

Re: myk's code contributions

Postby myk » 09 Mar 2013, 22:37

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?)
myk
 
Posts: 439
Joined: 17 Jan 2013, 02:39
Location: California
Has thanked: 38 times
Been thanked: 57 times

Re: myk's code contributions

Postby timmermac » 10 Mar 2013, 00:59

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?)
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.
"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
User avatar
timmermac
Tester
 
Posts: 1512
Joined: 17 May 2010, 20:36
Has thanked: 18 times
Been thanked: 95 times

Re: myk's code contributions

Postby myk » 10 Mar 2013, 01:19

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.
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.
Also, isn't the /res/pic_products folder supposed to be where the booster, starter and other images of that type go?
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.
IIRC, /res/quest/prices.txt is supposed to be the baseline for quest spell shop card prices, unless that got moved somewhere else.
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).

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)
given this, if you set cacheDir to <forgeDir>/res, the cards will just be moved from <forgeDir>/res/pics to <forgeDir>/res/pics/cards. If this is an issue, it won't be hard to declare a third configurable directory, cardPicsDir or somesuch, so you can define that to <forgeDir>/res/pics and keep them in exactly the same place as they are now.

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

Postby 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. :D

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

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 all- prices.txt file may not be needed as we can easily update the all-prices.txt file at various points in the future.
User avatar
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

Postby Xitax » 10 Mar 2013, 03:22

Is there a chance that this will fix the bug of not ever picking [land]1.jpg?
Xitax
 
Posts: 918
Joined: 16 May 2010, 17:19
Has thanked: 183 times
Been thanked: 133 times

Re: myk's code contributions

Postby 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

Postby Xitax » 10 Mar 2013, 04:48

Thanks, myk! Your contributions lately have been impressive.
Xitax
 
Posts: 918
Joined: 16 May 2010, 17:19
Has thanked: 183 times
Been thanked: 133 times

Re: myk's code contributions

Postby 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:
  1. 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.
  2. Download the snapshot build of my branch from http://cardforge.org/releases/snapshots/forge/branches/myk-separate-writable-data/current/
  3. Install it over the copy that you made
  4. 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

Postby 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
Programmer
 
Posts: 3486
Joined: 14 Mar 2011, 05:58
Has thanked: 683 times
Been thanked: 569 times

PreviousNext

Return to Developer's Corner

Who is online

Users browsing this forum: No registered users and 23 guests


Who is online

In total there are 23 users online :: 0 registered, 0 hidden and 23 guests (based on users active over the past 10 minutes)
Most users ever online was 4143 on 23 Jan 2024, 08:21

Users browsing this forum: No registered users and 23 guests

Login Form