New developer - trivial grammar bugfix patch inline

[dripton]

Hey guys. I just found Forge last week and have found a few bugs. I figured I should try to fix them. I read the new developer page, which said to click the box to ask for dev permissions, and got an email from Sloth telling me to post a patch here. So here's a simple one for an annoying little grammar nit (sorry if the forum software mangles whitespace):

$ svn diff
Index: src/main/java/forge/game/player/PlayerControllerHuman.java
===================================================================
--- src/main/java/forge/game/player/PlayerControllerHuman.java (revision 23110)
+++ src/main/java/forge/game/player/PlayerControllerHuman.java (working copy)
@@ -415,7 +415,11 @@
}

InputSelectCards inp = new InputSelectCardsFromList(min, max, valid);
- inp.setMessage(sa.hasParam("AnyNumber") ? "Discard up to %d cards" : "Discard %d cards");
+ if (max == 1) {
+ inp.setMessage(sa.hasParam("AnyNumber") ? "Discard up to %d card" : "Discard %d card");
+ } else {
+ inp.setMessage(sa.hasParam("AnyNumber") ? "Discard up to %d cards" : "Discard %d cards");
+ }
Singletons.getControl().getInputQueue().setInputAndWait(inp);
return inp.getSelected();
}

I currently have no ideas for big changes. Just a few little ones:

1. Fix bugs.

a. I reported a crash (a ConcurrentModificationException) in the forum. I'll try to hunt it down.

b. I also saw a crash (NullPointerException) but the forum software thought my traceback looked "too spammy for a new user" and wouldn't let me post it. I'll try to fix that one too.

c. The AI plays Seal of Fire poorly. I'd like it to be a bit more patient rather than chucking it at the player to reduce him from 20 life to 18.

2. Add a preferences checkbox to make foil effects optional. (Foil effects make the cards hard to read on my machine.)

[Chris H.]

Hello dripton.

I assume that you may have already found the getting started wiki at

How to Get Started Developing Forge


There should be a button which will allow you to attach the saved patch file to you message.


Once you have posted some number of messages (5?) you will find that you will no longer have that restriction and you will then be able to post the crash reports.

In the meanwhile you can save the crash report to your disk. You can then attach the txt file to your message.

[spr]

2. Add a preferences checkbox to make foil effects optional. (Foil effects make the cards hard to read on my machine.)

There is already a setting for this - see Game Settings - Preferences - Random Foil (in the Graphic Options section).

Nice spot regarding the non-closing of sound resources btw which may be the cause of http://www.slightlymagic.net/forum/viewtopic.php?f=26&t=11665.

Cheers,
Steve

[dripton]

I already have the Random Foil checkbox unchecked and a few cards still look weird. But I guess they could be accidentally-corrupted images rather than deliberately-differnet-looking foils. Or a bug with the checkbox. Or something else.

[spr]

I already have the Random Foil checkbox unchecked and a few cards still look weird. But I guess they could be accidentally-corrupted images rather than deliberately-differnet-looking foils. Or a bug with the checkbox. Or something else.

The foil effect is applied to the original unfoiled BufferedImage at runtime if the Random Foil setting is checked. See viewtopic.php?f=52&t=6333&start=2355#p130102 for what sounds like a similar issue - it may help you narrow down what is causing it.

Cheers,
Steve

[dripton]

Steve, that's exactly the bug I'm seeing. NPH and MBS cards have their colors all wrong inside the game, but look fine if loaded in an external image viewer. Other cards are fine. Nothing to do with foils after all, just some image files that Forge or a library it uses doesn't like. (On some systems.)

Edit: I ran ImageMagick's mogrify -antialias to load and re-save both versions of one of the NPH jpg files (the one in ~/.cache/forge/pics/cards and the one in ~/.cache/forge/pics/cards/NPH), and it fixed the problem. So I scripted doing that for all the NPH and MBS cards on my box. That doesn't help anyone else, though. If the problem is widespread then I can provide my fixed images to the download site. (Or the original source can re-process them, if they don't want artifacts from double JPEG compression.)

[spr]

Thanks dripton - looks like its related to a combination of the image format and OS. Maybe. I downloaded the LQ images and have no problems running in Windows 7.

Perhaps you could test the following to see whether this works. In ImageLoader.java (\src\main\java\forge), find the "_findFile" method and replace the line -

Code: Select all

   return ImageIO.read(file);

with

Code: Select all

   BufferedImage sourceImage = ImageIO.read(file);
   BufferedImage image = new BufferedImage(sourceImage.getWidth(), sourceImage.getHeight(), BufferedImage.TYPE_INT_RGB);
   Graphics2D g2d = (Graphics2D)image.getGraphics();
   g2d.drawImage(sourceImage, 0, 0, null);
   return image;

This is based on something I read at http://www.jhlabs.com/ip/managed_images.html which states

Watch out for ImageIO. The images returned by ImageIO are often in custom formats which can
draw really, really slowly. For best performance, it's often best to draw any image returned
by ImageIO into a new image of the appropriate pixel format for your system.

Worth a try.

Cheers,
Steve

[moomarc]

Perhaps you could test the following to see whether this works. In ImageLoader.java (\src\main\java\forge), find the "_findFile" method and replace the line

If that doesn't work, then I'll process those two sets with the script I use for all the new releases. Unfortunately that still won't help anyone with the issue unless they delete the offending picture sets and re-download.

Dripton, if you don't still have the images that were causing issues, let us know. We might be able to get Rob to try the patch.

[Max mtg]

$ svn diff
Index: src/main/java/forge/game/player/PlayerControllerHuman.java
===================================================================
--- src/main/java/forge/game/player/PlayerControllerHuman.java (revision 23110)
+++ src/main/java/forge/game/player/PlayerControllerHuman.java (working copy)
@@ -415,7 +415,11 @@
}

InputSelectCards inp = new InputSelectCardsFromList(min, max, valid);
- inp.setMessage(sa.hasParam("AnyNumber") ? "Discard up to %d cards" : "Discard %d cards");
+ if (max == 1) {
+ inp.setMessage(sa.hasParam("AnyNumber") ? "Discard up to %d card" : "Discard %d card");
+ } else {
+ inp.setMessage(sa.hasParam("AnyNumber") ? "Discard up to %d cards" : "Discard %d cards");
+ }
Singletons.getControl().getInputQueue().setInputAndWait(inp);
return inp.getSelected();
}

This is a poor fix.

1. You are copy-pasting the whole line instead of assigning only the variable that depends on maximum number.
2. When you are asked to discard up to 2 cards, after you discard one, you'll see message "Discard up to 1 cards" anyway.
3. A propper fix would be set up around a call to forge.util.Lang.nounWithAmount(int, String) from the very input (forge.gui.input.InputSelectManyBase.getMessage()) or an overload for this method implemented in the InputSelectCards class.

[Sloth]

Membership granted. You have now commit rights dripton.

Welcome on board.

[dripton]

Max: ack, I'll make a better version of that grammar fix (and test it for several duels against decks with discarders) before submitting it.

Marc / Steve: I still have the original images for those 2 sets, and will try the suggested tweak to the image code tonight.

[dripton]

Steve, I tried your suggested Image fix. It didn't work.

[spr]

Steve, I tried your suggested Image fix. It didn't work.

Thanks for trying anyway. Probably best to go with Marc's solution then and maybe add a sticky to the main forum.

I would still be interested to know what it is about the image that is different from one that does work and why re-processing solves the issue - resolution, bit depth, compression, etc? But then why does it work on my Windows 7 PC and Linux Zorin VirtualBox? Considering there have been over 4000 downloads I would have expected more complaints than the two I am aware of. Perhaps it is some obscure JVM config setting?

Cheers,
Steve

[Rob Cashwalker]

Dripton, if you don't still have the images that were causing issues, let us know. We might be able to get Rob to try the patch.

I'll try it.. this is driving me nuts...
I'm glad someone figured this out.

[spr]

Some more info (mentions jpg headers and CMYK formats ) -
- http://stackoverflow.com/questions/9340569/jpeg-image-with-wrong-colors
- http://stackoverflow.com/questions/13072312/jpeg-image-color-gets-drastically-changed-after-just-imageio-read-and-imageio
- http://stackoverflow.com/questions/4386446/problem-using-imageio-write-jpg-file?lq=1

Cheers,
Steve