Encapsulate Quest Opponents in Deck Files

[Max mtg]

OK, so the structure

Code: Select all

[metadata]
[main]
[sideboard]
[ai_extra_cards]
[human_extra_cards]

is now acceptable?

The file structure is good enough.

And, those fields are retrieved by the deck manager class?

DeckManager should read only [main], [side] and four fields from [metadata], so it does now.
All others sections are better to be retrieved in QuestUtil or a class like that.

And, may "illegal" tokens be stored there with the pattern I described? (as long as they are processed by a non-deck class, like QuestUtil)

Yes, at any of your newly added sections.
And the code that is going to read that tokens and cards that Wizards never printed - it should store them into CardList classes, not CardPools. Quest_Assignment class is the 1st candidate to become a model to store that data.

where Deck has fixed fields instead of too flexible metadata map

Not sure what you mean by this? It was sounding like the flexible meta was a positive thing?

It's just to avoid having a bad design.

[friarsol]

where Deck has fixed fields instead of too flexible metadata map

Not sure what you mean by this? It was sounding like the flexible meta was a positive thing?

It's just to avoid having a bad design.

Since the Deck class is designed to be limited in scope, the fields were removed because flexible metadata fields are unnecessary (and can be handled elsewhere). We know that Decks will contain a pool of cards for the mainboard and a pool of cards for the sideboard, and that's it. It has no need for more Flexible structures.

[Doublestrike]

DeckManager should read only [main], [side] and four fields from [metadata], so it does now....should store them into CardList classes, not CardPools. Quest_Assignment class is the 1st candidate to become a model to store that data.

OK great, hugely helpful. Can do.

Since the Deck class is designed to be limited in scope, the fields were removed because flexible metadata fields are unnecessary (and can be handled elsewhere.

Also very helpful, thanks for taking the time to explain.

===========

OK then, if it's OK, I'll get to work on this soon (hopefully today if not too busy). Also, if nobody minds, I'll put in documentation into "deck" that summarizes this discussion.

[Doublestrike]

Finally got some time away from Day Job. Requested changes completed in r10357, bug 4 resolved.

All dck files now have only standard metadata.

Quest-related data is held in quests.txt.

The hard-coded data in QuestUtil has been removed.

A couple of questions:

1. GameAction.getComputerPlayer() and a few other methods were notated as "will eventually be marked deprecated". Is there an option in place to use instead? Will update to that if so.

2. While creating tokens, I wasn't sure about card type handling regarding CapiTaliZation. Is there a problem on *nix systems with capitalization, or has this been handled?

3. I assume card duplicate checking happens somewhere? There were a few lines about it from the last person to work on these files (ReadQuest_Assignment, I think) but it seems to me that's not the correct place to do card checking.

4. Are both Windows and *nix newlines acceptable? That is, both \r\n and \n work? Personally, as I'm on a Windows box, I'd prefer the \r\n since editing files becomes much easier for users. However, I've made everything \n at present (can change easily).

=====

Have also scrapped all of my work from previous effort. Actually, a lot of it is still quite valid, but I need to commit in smaller steps - I had about twenty revisions worth of work, in one. Am updating development wiki to indicate that for users new to SVN.

[Doublestrike]

By the way, the deck files aren't completely standard perfect. Working on that now.

[friarsol]

Hey Doublestrike,
Any reason you decided to keep quests.txt around instead of putting all of the quest meta data in the one file? Then any .qst file in the decks folder would be picked up and parsed, so new quests could be added without needing to change two files, or if a player really disliked a certain quest they could remove it. It's definitely an improvement, but why not go all the way to a single file per quest?

Something like this:

quest1.qst | Open

[quest]
Icon=Dungeon Crawling White.jpg
Title=Dungeon Crawling (White)
Desc=This realm is guarded by a divine entity laying his protecting hand on each living being.
Diff=Medium
AILife=25
Repeat=true
Wins=20
Card Reward=3 white rares
Credit Reward=100
HumanExtras=
AIExtras=Divine Presence
[metadata]
Name=quest1
DisplayName=Dungeon Crawling (White)
Icon=Dungeon Crawling White.jpg
Deck Type=constructed
Comment=
[main]
24 Plains
1 Abbey Gargoyles
1 Archon of Redemption
1 Border Guard
1 Cho-Arrim Bruiser
1 Commander Eesha
1 Daru Lancer
1 Femeref Scouts
1 Foot Soldiers
1 Fountain Watch
1 Harvest Gwyllion
1 Hearthfire Hobgoblin
1 Herald of Serra
1 Hero of Bladehold
1 Linvala, Keeper of Silence
1 Makindi Griffin
1 Mirran Crusader
1 Peregrine Griffin
1 Spiritual Guardian
1 Standing Troops
1 Starlit Angel
1 Staunch Defenders
1 Teroh's Faithful
1 Warrior Angel
1 Wilt-Leaf Cavaliers
1 Zealots en-Dal
1 Beckon Apparition
1 Blessed Reversal
1 Chastise
1 Condemn
1 Disempower
1 Excommunicate
1 Last Breath
1 Narrow Escape
1 Rebuff the Wicked
1 Retribution of the Meek
1 Sunlance
[sideboard]

[Doublestrike]

@friarsol - OK will change. After re-re-reading this forum, it looks like I misunderstood what everyone had in mind.

@everyone else - please speak up if this isn't what you envisioned

[friarsol]

@friarsol - OK will change. After re-re-reading this forum, it looks like I misunderstood what everyone had in mind.

@everyone else - please speak up if this isn't what you envisioned

No worries. I think our general consensus was a single file is cool, just have both classes Parse them separately.

[Doublestrike]

Great, that's what I was shooting for the first time around. As I was writing this second version, I kept thinking, "this is a dumb way to do it", so I'm happy to do it properly.

Third time's a charm.

[Max mtg]

Generally I like how you've made it.

Your idea of reading a single fiel with metadata and then read only the requested deck is good, but unfortunatelly this creates references between files and makes them harder to edit, so I have to agree with Sol about encapsulation into one file.

Another point - you don't have to keep buffered reader open in ReadQuest_Assignment.run(). Instead you can read the file with FileUtil.readFile(file), get a List<String> from it and iterate over it, so that your parser code does not need to be warped into a try/catch for IOExceptions.

And why is ReadQuest_Assignment a Runnable? Is it executed in a dedicated thread somewhere?
In ReadQuest_Assignment I've seen some duplicate code:

Code: Select all

                        String[] names = linedata[1].split("\\|");
                        CardList templist = new CardList();
                        Card tempcard;
                       
                        for(String s : names) {
                            // Token card creation
                            if(s.substring(0,5).equals("TOKEN")) {
                                tempcard = QuestUtil.createToken(s);
                                tempcard.addController(AllZone.getHumanPlayer());
                                tempcard.setOwner(AllZone.getHumanPlayer());
                                templist.add(tempcard);
                            }
                            // Standard card creation
                            else {
                                tempcard = AllZone.getCardFactory().getCard(s, AllZone.getHumanPlayer());
                                tempcard.setCurSetCode(tempcard.getMostRecentSet());
                                tempcard.setImageFilename(CardUtil.buildFilename(tempcard));
                                templist.add(tempcard);
                            }
                        }

This might be just a single method.

You are comparing strings using equals, I think compareIgnoreCase would be better so that case typos won't break the parser (was it AIExtras or AiExtras ?)

It would be nice as well to move these two files ReadQuest_Assignment and Quest_Assignment itself into package forge.quest

[Doublestrike]

encapsulation into one file.

Great, will do.

Another point - you don't have to keep buffered reader open in ReadQuest_Assignment.run(). Instead you can read the file with FileUtil.readFile(file), get a List<String> from it and iterate over it, so that your parser code does not need to be warped into a try/catch for IOExceptions.

Thanks for the tip - that's one of those Java things I've got to pick up...handled differently in PHP.

And why is ReadQuest_Assignment a Runnable? Is it executed in a dedicated thread somewhere?

I was wondering that myself. Apparently, when ReadQuest_Assignment is instantiated, a separate .run() method must be called from it. This is how The Person Before Me did it, so I let sleeping dogs lie (didn't touch it). But I'll go ahead and factor it out in the next version.

In ReadQuest_Assignment I've seen some duplicate code...

That duplicate code assigns the same tokens to the human, or the AI. The token creation is handled in the same place, in QuestUtil, then attached to the card list after controller assignment.

You are comparing strings using equals, I think compareIgnoreCase would be better so that case typos won't break the parser (was it AIExtras or AiExtras ?)

Also great to know, didn't know that existed.

It would be nice as well to move these two files ReadQuest_Assignment and Quest_Assignment itself into package forge.quest

Completely agree. This was one of the things done in the infamous r10194 (?) but it was lost. Will of course do again. There's some other restructuring that must happen too - some Swing mixed into the model, some poorly named and poorly documented stuff. I'll be cranking away slowly.

[Max mtg]

In ReadQuest_Assignment I've seen some duplicate code...

That duplicate code assigns the same tokens to the human, or the AI. The token creation is handled in the same place, in QuestUtil, then attached to the card list after controller assignment.

So what was preventing you from eliminating that duplicate by passing a parameter the method you were about to create?
like this: http://svn.slightlymagic.net/websvn/dif ... &peg=10501