Page 5 of 6

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 22 Dec 2009, 07:05
by Huggybaby
Why is renaming necessary? The ones I uploaded were downloaded using Forge.

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 22 Dec 2009, 07:06
by nantuko84
how about checking at the end of each turn whether an image is currently shown on the screen, if not, kick it out of cache?
I do believe caching ONE picture is not the problem, so just remember the latest seen card picture - the main point here is not to store all pictures in memory

I've looked into the code: first, you need to update this line in ImageCache.java:
Code: Select all
cache.put(name, resized);
In MagicWars I use more or less the same code (it will be strange if I don't as ImageCache came from MW 8) ), but if cache.size() gets 30, I remove 10 images from it.
Take into account that reseting cache doesn't mean that all card images will be removed from battlefield, it is used just to reduce time for resizing every image in the game.
What can I say here, as this image is resized before putting into cache, it doesn't matter what quality (HQ or not) it was - so the issue didn't come from here.

Second suggestion is about how full image is displayed:
sorry, but it's really awful ;( let me show you the call stack:
MouseMotionAdapter->updateCardDetail->picturePanel.removeAll()+picturePanel.add(pic)->
getPicture(Card c)->new PicturePanel(file)->new ImageIcon(f.getPath())

so on every mouse move! you remove previous JPanel and create new one (even if it's the same picture). no need to say that it has low performance
at least you may
1. separate displaying image and card details (text)
2. cache what card has been seen last
3. may be cache limit amount of images not to read them from disk every time

and for sure no need to remove and create so many objects every time, you can change image directly on component (here is ImageIcon) and make picturePanel invisible if you leave its area

regards

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 22 Dec 2009, 14:57
by DennisBergkamp
Huggybaby wrote:Why is renaming necessary? The ones I uploaded were downloaded using Forge.
Ah yes, but in the current public release, pics.slightlymagic.com can't be used yet as a mirror (the next version will have it).

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 22 Dec 2009, 15:25
by DennisBergkamp
nantuko84 wrote:
how about checking at the end of each turn whether an image is currently shown on the screen, if not, kick it out of cache?
I do believe caching ONE picture is not the problem, so just remember the latest seen card picture - the main point here is not to store all pictures in memory

I've looked into the code: first, you need to update this line in ImageCache.java:
Code: Select all
cache.put(name, resized);
In MagicWars I use more or less the same code (it will be strange if I don't as ImageCache came from MW 8) ), but if cache.size() gets 30, I remove 10 images from it.
Take into account that reseting cache doesn't mean that all card images will be removed from battlefield, it is used just to reduce time for resizing every image in the game.
What can I say here, as this image is resized before putting into cache, it doesn't matter what quality (HQ or not) it was - so the issue didn't come from here.

Second suggestion is about how full image is displayed:
sorry, but it's really awful ;( let me show you the call stack:
MouseMotionAdapter->updateCardDetail->picturePanel.removeAll()+picturePanel.add(pic)->
getPicture(Card c)->new PicturePanel(file)->new ImageIcon(f.getPath())

so on every mouse move! you remove previous JPanel and create new one (even if it's the same picture). no need to say that it has low performance
at least you may
1. separate displaying image and card details (text)
2. cache what card has been seen last
3. may be cache limit amount of images not to read them from disk every time

and for sure no need to remove and create so many objects every time, you can change image directly on component (here is ImageIcon) and make picturePanel invisible if you leave its area

regards
Thanks for your reply, it seems you know a lot more about this than I do :) I think I'm a bit lost though... #-o What do you mean by "I do believe caching ONE picture is not the problem"?
What I did as a test, is to loop through all of the image names that are currently in cache, and compare them with all of the cards that are currently in play or in the player's hand (which should be a good indication of whether they're visible on the screen). If they're not there, dump the missing ones from cache... and this could be done at the end of every turn (or heck, multiple times a turn). Is this a good idea, or is it not even worth it? At least it seems the relevant images are getting removed.
This is the (really ugly) code:

Code: Select all
CardList visibleCards = new CardList();
             PlayerZone hPlay = AllZone.Human_Play;
             PlayerZone hand = AllZone.Human_Hand;
             PlayerZone cPlay = AllZone.Computer_Play;
            
             visibleCards.addAll(hPlay.getCards());
             visibleCards.addAll(hand.getCards());
             visibleCards.addAll(cPlay.getCards());
            
             ArrayList<String> list = new ArrayList<String>();
            
             Iterator<String> iter = ImageCache.cache.keySet().iterator();
             while(iter.hasNext()) {
                 String cardName = iter.next();   
                 if ( !(cardName.startsWith("Swamp") || cardName.startsWith("Mountain") || cardName.startsWith("Island")
                    || cardName.startsWith("Plains") || cardName.startsWith("Forest")) &&
                    visibleCards.getImageName(cardName).size() == 0 ) {
                    list.add(cardName);
                 }
             }
            
             for (String s : list)
             {
              ImageCache.cache.remove(s);
              System.out.println("Removing " + s + " from cache.");
             }
And basically what you're saying is, ImageCache.cache is currently used to cache just the resized images ? (To save some time because resizing is expensive). Also that would mean the resolution of the images wouldn't matter, since they're resized to a smaller (fixed) resolution.
This would also mean that the full pics on the right are not cached. And this is what confuses me, since we haven't had any Heap space problems before the HQ pictures update. Do they get cached somewhere else?

Hmm, I didn't know the current way of displaying pictures was so inefficient... would there be an easy way to maybe implement it the same way you did in MagicWars :mrgreen: ?

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 22 Dec 2009, 16:24
by Rob Cashwalker
Wasn't it nantuko who originally provided the in-play mini card pic function?

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 23 Dec 2009, 03:58
by freestorageaccount
About changing the heap allocation, I tried for a permanent fix by adjusting my Java control panel settings, but unfortunately there was no option for heap size! Must I override the default each time I run Forge?

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 23 Dec 2009, 11:01
by Hamletchickencrisis
Aluren seems to be unusable by both sides despite the text.

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 23 Dec 2009, 18:47
by DennisBergkamp
freestorageaccount wrote:About changing the heap allocation, I tried for a permanent fix by adjusting my Java control panel settings, but unfortunately there was no option for heap size! Must I override the default each time I run Forge?
Check out this link:
http://www.duckware.com/pmvr/howtoincre ... emory.html

They use -Xmx300, you could of course set it to 512, to be sure :)

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 23 Dec 2009, 23:26
by freestorageaccount
Hmm, I still get a lot of errors. Maybe those flags aren't being used every time or I should set them to a few gigs, even though your site said it might not work.

This isn't the bugs thread, but I thought I'd note this here to consolidate it into one post: If you play a Mox (any other spell or ability placed on the stack would probably work as well, perhaps even a turning-face-up action since it's currently implemented as such, but I did this using a Mox Sapphire), you can respond to yourself with a land! Doing so brings up a dialog box with a single option, 'Play land', even though a land can be played only as a sorcery (and at most once during your turn). I'm not sure if this is intentional, but if so, it must have been some trouble to go through to implement an action that can't actually be done.

EDIT: In addition to the weird oscillations from the exalted ability when an attacker's companions leave combat, clicking many times on a single attacker when you're prompted to declare your attackers has the same effect (at least in the combat panel to the left of the main playing area).

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 23 Dec 2009, 23:45
by DennisBergkamp
What kind of errors are you getting, still the same heap space related ones?

I know about the Land bug, I fixed it a few days ago in the SVN (you can actually cast lands during the AI's turn too, in response to him casting a spell/ability).
It came about because I added functionality for Cycle and Transmute for lands...

For Exalted, you mean a creature can get multiple exalted bonuses by just declaring it as an attacker multiple times? I will rewrite exalted soon, where it's just a triggered ability that gets checked once AFTER attackers have been declared.

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 24 Dec 2009, 01:15
by freestorageaccount
Yes, the heap space ones. I have a feeling that :twisted: is trying to prevent me from playing Forge. :(

Clicking on an attacker repeatedly causes its power/toughness bonus from exalted (as it's implemented now; technically it's given as a triggered ability immediately after attackers are declared, still in the 'declare attackers' step, as you observed) to oscillate in the combat panel. For example, you can attack with your Noble Hierarch and make it flicker between 1/2 and 0/1, just like in the other exalted bug. I don't know whether this has an effect or is cosmetic, and I haven't tried it with multiple exalted effects.

A trivial note: You actually may pay your last life point (but no more); I just played a Hallowed Fountain at precisely two and wasn't given the option to prevent it from entering tapped due to its own effect, which might matter if Platinum Angel is implemented (had I been at one life, however, not presenting the choice would have been correct). I think Necropotence currently allows your life to go negative without any O:) intervention, which is impossible if state-based effects are run as often as they should be. (Relatively lengthy thought bubble containing sub-bubbles: On the other hand, you can cast Psionic Blast regardless of how many life points you have [or don't have], since its announcement isn't contingent upon your life, and as far as the game knows, any number of things could happen between that announcement and resolution, which would make forbidding playing it at certain times impractical [what if you're at two life but also have a Crystal Rod...?].)

Even if you guys do add the excellent Platinum Angel, though, I couldn't play with it in my quest. I already disallow myself from running Stuffy Doll and the 10 Lightning Bolt deck since that would nearly be cheating.

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 24 Dec 2009, 03:48
by Resonantg
Counterspell also doesn't seem to be working, unless I'm just using it wrong.

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 24 Dec 2009, 07:18
by nantuko84
Code: Select all
Wasn't it nantuko who originally provided the in-play mini card pic function?
seems so. I remember I created displaying images on battlefield (by not in picture panel), image cache and rotating them for tapped permanents.
What can I say now is that image caching is not big problem here and not the cause of jave heap exception. First, all images are resized before being put to cache, the second - usually amount of such pictures is not big, but anyway you may limit cache size.

What I did as a test, is to loop through all of the image names that are currently in cache, and compare them with all of the cards that are currently in play or in the player's hand (which should be a good indication of whether they're visible on the screen). If they're not there, dump the missing ones from cache... and this could be done at the end of every turn (or heck, multiple times a turn). Is this a good idea, or is it not even worth it? At least it seems the relevant images are getting removed.
Sorry, but it would break the idea of caching.
If u play Birds of Paradise, then it gets removed by Terror, next time you play new Birds of Paradise, its resized image should be taken from image cache, so no need to remove them at the end. As I said before, you may add checking for amount, smth like:
Code: Select all
if (cache.size() == 30) {
   // remove here first 10 leaving 20 pictures in cache
}
Oh, forgot to say: first you need to reproduce the error somehow.
If you don't know such steps, then after any fix, you won't be able to check whether it was really fixed or not.

And once again: it won't solve your outofmemory exception. One day I've added logging to picture panel and found out that during common game it was called 1000 times (just multiply it by HQ image size). You said that exception had appeared after starting using HQ pictures. It makes sense: though whenever you create new picture panel, old becomes unused and will be removed from memory by garbage collector, it won't happen immediately, and I can image the situation when there will be too many ungarbaged objects and java says it has not enough memory to create new one.

So, just reimplement displaying pictures and I do believe exception will be thrown away.

Regards

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 24 Dec 2009, 20:33
by DennisBergkamp
Ok, that makes more sense. There's just too much garbage imagepanels floating around that clog up memory. I'll see if I can figure out how to fix the way full-sized images are shown.

Re: Forge 12/14 (unofficial BETA) version

PostPosted: 24 Dec 2009, 20:35
by DennisBergkamp
Resonantg wrote:Counterspell also doesn't seem to be working, unless I'm just using it wrong.
Counterspell should work just fine, just cast it whenever a spell is on the stack and the spell should be countered.