Page 2 of 3

Re: Cleanup and hoping to increase performance

PostPosted: 31 Jul 2012, 02:13
by friarsol
Looks like cards that start in play via Quests aren't being run through the cardfactory creation.

Eladamri's Vineyard isn't triggering at all from the Green Medium Quest. If I cast a Vineyard in constructed play, it triggers just fine.

Re: Cleanup and hoping to increase performance

PostPosted: 31 Jul 2012, 07:44
by Sloth
It's really fast now on my slow computer (not just the loading time, but during matches as well). I'm sure some of our users will be very happy about this. :D

Thumbs up Max.

Re: Cleanup and hoping to increase performance

PostPosted: 31 Jul 2012, 10:34
by Max mtg
friarsol wrote:Looks like cards that start in play via Quests aren't being run through the cardfactory creation.

Eladamri's Vineyard isn't triggering at all from the Green Medium Quest. If I cast a Vineyard in constructed play, it triggers just fine.
Yes, that must be related to cards that are placed on battlefield before the match starts. I'll check it todays evening.

Sloth, I would also appreciate if you pressed that special button (in the top-right corner of the post frame) ;)


mcrawford620, please, consider refactoring LimitedDeck to use CardPrinted instances, not forge.Card(s)

Re: Cleanup and hoping to increase performance

PostPosted: 31 Jul 2012, 13:33
by Chris H.
Using today's snapshot build I see that the LQ pic downloading code wants to download over 2000 pics yet I should be up to date.

Let me run a test and see what happens.

Re: Cleanup and hoping to increase performance

PostPosted: 31 Jul 2012, 13:47
by Chris H.
Chris H. wrote:Using today's snapshot build I see that the LQ pic downloading code wants to download over 2000 pics yet I should be up to date.

Let me run a test and see what happens.
 
It is downloading three different forest pics and is naming them:

1.jpg
2.jpg
3.jpg

As the code downloads the 2403 pics it does not stop after writting these three files. It continues to download the same three pics and will replace the previously named pics with another copy.

After a period of time I see that these three files are no longer forest pics but it is now island pics. I assume that I will see mountains, plains and swamps before this is finished. :)



EDIT:

Once we have the three files named:

1.jpg
2.jpg
3.jpg

and then click on the download LQ pics button we will be told that all pics have been downloaded.

Re: Cleanup and hoping to increase performance

PostPosted: 31 Jul 2012, 17:38
by mcrawford620
Max mtg wrote:mcrawford620, please, consider refactoring LimitedDeck to use CardPrinted instances, not forge.Card(s)
I'd be glad to take a look.

It looks like I should start from BoosterDraft.java? The sequence runs:
- BoosterDraft already has CardPrinted, but converts to Cards to send to BoosterDraftAI
- BoosterDraftAI picks from Cards and puts them into CardLists
- then sends them on to LimitedDeck to build the deck

The Booster and Limited code uses a lot of the CardList methods to filter down the available cards, based on colors, type of card, and so on. How should I go about refactoring those methods?

Re: Cleanup and hoping to increase performance

PostPosted: 31 Jul 2012, 18:45
by Max mtg
mcrawford620 wrote:The Booster and Limited code uses a lot of the CardList methods to filter down the available cards, based on colors, type of card, and so on. How should I go about refactoring those methods?
Booster draft AI AFAIR has not been refactored since CardPrinted introduction, yet it would be great to do so.

There are methods to filter CardPrinted instances:
Code: Select all
Itereable<CardPrinted> cards = CardDb.instance().getAllCards() // or whatever the source collection is;

// colors
List<CardPrinted> redCards = CardRules.Predicates.Presets.IS_RED.select(cards, CardPrinted.FN_GET_RULES);

// Type of cards
List<CardPrinted> creatures = CardRules.Predicates.Presets.IS_CREATURE.select(cards, CardPrinted.FN_GET_RULES);
List<CardPrinted> spells = CardRules.Predicates.Presets.IS_NONCREATURE_SPELL.select(cards, CardPrinted.FN_GET_RULES);

// AI playability
List<CardPrinted> goodForRandomDecks = CardRules.Predicates.IS_KEPT_IN_RANDOM_DECKS.select(cards, CardPrinted.FN_GET_RULES);
For more examples refer to multi-colored deck generators in forge.deck.generate package.
I will also be able to answer your questions right here (or in a dedicated thread)


Chris H. wrote:Using today's snapshot build I see that the LQ pic downloading code wants to download over 2000 pics yet I should be up to date.

Let me run a test and see what happens.
Chris, this should be no longer an issue.

Re: Cleanup and hoping to increase performance

PostPosted: 31 Jul 2012, 18:55
by Sloth
Max mtg wrote:
friarsol wrote:Looks like cards that start in play via Quests aren't being run through the cardfactory creation.

Eladamri's Vineyard isn't triggering at all from the Green Medium Quest. If I cast a Vineyard in constructed play, it triggers just fine.
Yes, that must be related to cards that are placed on battlefield before the match starts. I'll check it todays evening.
I want to point out that devSetupGameState() basically does the same thing (putting permanents onto the battlefield) and works fine. Maybe the quest setup can share some of its code.

Re: Cleanup and hoping to increase performance

PostPosted: 31 Jul 2012, 19:07
by mcrawford620
Looks good, thanks for the help. I'll do what I can; not sure if I can finish by the end of the week.

It looks like CardPrinted has a CardRules, which seems to contain everything about a card? What about the actual Card object is so heavy?

Re: Cleanup and hoping to increase performance

PostPosted: 31 Jul 2012, 19:40
by friarsol
mcrawford620 wrote:Looks good, thanks for the help. I'll do what I can; not sure if I can finish by the end of the week.

It looks like CardPrinted has a CardRules, which seems to contain everything about a card? What about the actual Card object is so heavy?
Don't worry about finishing by the end of the week. Just finish when you can.

Card objects are basically full Game objects with Abilities and Triggers etc attached.

Re: Cleanup and hoping to increase performance

PostPosted: 31 Jul 2012, 19:44
by Max mtg
mcrawford620 wrote:It looks like CardPrinted has a CardRules, which seems to contain everything about a card? What about the actual Card object is so heavy?
you are welcome :)

Exactly. CardPrinted is created for each variant of card printed in each set (4 islands in M13, 4 for M12, 2 from NPH and so on). This class is great for collection management. Each CardPrinted contains name, edition, rariry, picture index and a reference to CardRules object.
There is one and only CardRules object for each card name, regardless of number of its reprints. Here all the basic information is stored - type, color, manacost, oracle text (it is not parsed here into triggers or abilities). This class is good for deckbuilding.

Why is Card so heavy? You'd better have a look at the forge.Card class declaration :) It contains just everything a card might experience during game - counters, damage, changes of everyting until EOT, chosen colors, types, etc. Unlike CardPrinted and CardRules which are supposed to be immutable and exist as a single instance for each card, the forge.Card instances were copied with all of their fields when someone requested some Cards from the factory, and that was a source of slowdowns.

At the moment there are no Card instances when the game starts, that allows it to start a bit faster than before and cosume less memory.

Re: Cleanup and hoping to increase performance

PostPosted: 31 Jul 2012, 21:57
by Max mtg
Sloth wrote:I want to point out that devSetupGameState() basically does the same thing (putting permanents onto the battlefield) and works fine. Maybe the quest setup can share some of its code.
This problem emerges due to Card.resetUniqueNumber() call performed after cards to lay on the table at the beginning of a match had already been created (these were given ids 1-5 for instance), but before players' decks were to be materialized from CardPrinted. After that reset, all the new Cards recieve IDs starting from 1 again. In this manner we get a number of cards with same IDs.

In the mentioned case trigger on Eladamri's Vineyard (id=4) checks card's zone. To learn its zone, our code iterates over all players' zones looking for a card with Id = 4 there and finds a (goblin guide) with id = 4 in human's library. The code conclues that Eladamri's Vineyard is in a unsiutable place for the trigger to work and cancels it.

Re: Cleanup and hoping to increase performance

PostPosted: 01 Aug 2012, 06:10
by mcrawford620
It went more easily than I expected, I checked it in and it works. I left some little bits out for now -- I wasn't sure of the best way to use the SVars that I had been using from the Card objects. Add something to CardCharacteristics? Not sure.

Re: Cleanup and hoping to increase performance

PostPosted: 01 Aug 2012, 06:58
by Max mtg
mcrawford620 wrote:It went more easily than I expected, I checked it in and it works. I left some little bits out for now -- I wasn't sure of the best way to use the SVars that I had been using from the Card objects. Add something to CardCharacteristics? Not sure.
Which exactly svars do you need? Will
Code: Select all
forge.card.CardRules.getDeckWants()
do the job?

Re: Cleanup and hoping to increase performance

PostPosted: 01 Aug 2012, 16:34
by mcrawford620
Perfect, thanks.
I go back and forth on whether this whole DeckWants thing is worth it but for now I'll keep it in there. I underestimated how many cards 11,000 is.
Ultimately I think Rob's idea in viewtopic.php?f=52&t=7510#p93635 is better for deckbuilding but it's a long way off.