Big commit, no dice / SVN hugely broken on r10194

[Doublestrike]

Of course, different class is needed. And it should not inherit, but include the deck, and avoid storing metadata in hashtable, since the entries of it have different meaning.

Yep, I don't care what happens, just doing what the voices are telling me.

For now, I'll fix the stuff from r10194, and perhaps this input from Max mtg could be a new issue in the bug tracker.

[Max mtg]

Of course, different class is needed. And it should not inherit, but include the deck, and avoid storing metadata in hashtable, since the entries of it have different meaning.

Yep, I don't care what happens, just doing what the voices are telling me.

For now, I'll fix the stuff from r10194, and perhaps this input from Max mtg could be a new issue in the bug tracker.

Then let it be my voice telling you

You won't be able to fix that by inserting into the deck class two extra cardpools for cards on battlefield. Cardpools do not support fictional cards, even tokens. So, your quest assignments will fail to load. You'll have to figure out a data structure for those extensions now.

[Doublestrike]

You'll have to figure out a data structure for those extensions now.

Sure, but since my last efforts at figuring out a data structure weren't exactly well-received, perhaps you could tell me what you think would be a good solution.

Frankly, it seems like just rolling back my changes is a good idea.

[Doublestrike]

Cardpools do not support fictional cards, even tokens.

Well, since the project has, and will have, decks made with fictional cards or tokens, it seems like something is lacking in CardPool - is this the data structure you're referring to?

[Max mtg]

Cardpools do not support fictional cards, even tokens.

Well, since the project has, and will have, decks made with fictional cards or tokens, it seems like something is lacking in CardPool - is this the data structure you're referring to?

Why are you so sure? Forge had never had fictional cards in decks, niether tokens. They are not legal by mtg rules.

Why don't you want to implement your changes in a correct way? That is without even touching decks or cardpool.

[Doublestrike]

BTW I did fix the deck manager. It just ignores the tokens in quests, for now. I'm not sure if a token can be added to the card pool (or even if that's "allowed") but if it's possible, there's already a method in QuestUtil that reads the card entry and creates a token Card.

============

There's some other errors in GameAction and others that have to do with counting the cards in the deck - renumbering, foiling, etc. Apparently the cards can't be accessed by index any more? Not sure if this is a svn sync problem or what, but I didn't alter that during my work.

[Max mtg]

You'll have to figure out a data structure for those extensions now.

Sure, but since my last efforts at figuring out a data structure weren't exactly well-received, perhaps you could tell me what you think would be a good solution.

Frankly, it seems like just rolling back my changes is a good idea.

I've already told you that a separate class containing data related to the challenege and a deck for ai would be a solution, with some serialization added.

Rollback would be fair, at least for the time being while you are fixing the build.

[Max mtg]

BTW I did fix the deck manager. It just ignores the tokens in quests, for now. I'm not sure if a token can be added to the card pool (or even if that's "allowed") but if it's possible, there's already a method in QuestUtil that reads the card entry and creates a token Card.

============

There's some other errors in GameAction and others that have to do with counting the cards in the deck - renumbering, foiling, etc. Apparently the cards can't be accessed by index any more? Not sure if this is a svn sync problem or what, but I didn't alter that during my work.

It's strictly forbidden to add tokens into deck, by design of CardPool and by rules of MTG.
However challenges need that cards in play as a significant part of the quest =)

Just rollback if you cannot fix that.

[Doublestrike]

Why are you so sure? Forge had never had fictional cards in decks, niether tokens. They are not legal by mtg rules.

I'm sure because they're existing, right now, in the published build. Quests use tokens. MTG rules also state 20 life at the start, but that's also been ignored.

Regarding fictional cards and other uses, this forum has had discussions about puzzle modes, gauntlets, Shandalar clones, etc. which could very well use some fake cards. Also, users might want to make their own cards - why not?

Why don't you want to implement your changes in a correct way?

Why would you say this? I'm trying very hard to fix a problem that is months old. I'm not trying to screw up your project, I'm trying to improve it. If you give me a correct way, as I've said, I'll do it that way. This deckmanager cardpool change is relatively new, and happened during my changes to deckmanager as well.

That is without even touching decks or cardpool.

The bug description says "Encapsulate Quest Opponents in Deck Files". How am I supposed to do that without touching a deck?

[Doublestrike]

I've already told you that a separate class containing data related to the challenege and a deck for ai would be a solution, with some serialization added.

And I've already told you that "For now, I'll fix the stuff from r10194, and perhaps this input from Max mtg could be a new issue in the bug tracker."

This sounds a lot like hard-coding the data into a class. That's already in place, in QuestUtil. I thought we were trying to get away from that.

Rollback would be fair, at least for the time being while you are fixing the build.

Sure, should I do that, or do you want to? I'll probably screw something up.

Anyway, you can just throw out the work if you want, I guess. If it's so horrible then no reason to keep it around.

[Max mtg]

I'm sure because they're existing, right now, in the published build. Quests use tokens. MTG rules also state 20 life at the start, but that's also been ignored.

Regarding fictional cards and other uses, this forum has had discussions about puzzle modes, gauntlets, Shandalar clones, etc. which could very well use some fake cards. Also, users might want to make their own cards - why not?

Why would you say this? I'm trying very hard to fix a problem that is months old. I'm not trying to screw up your project, I'm trying to improve it. If you give me a correct way, as I've said, I'll do it that way. This deckmanager cardpool change is relatively new, and happened during my changes to deckmanager as well.

The bug description says "Encapsulate Quest Opponents in Deck Files". How am I supposed to do that without touching a deck?

Looks like you never played magic with real cards. How is one supposed to cast a token from a deck? That's absurd.
Quests put tokens directly onto the battlefield, this is different than storing them in a deck.

Yes, there's a lot of dreams and concepts and a few developers. But, let's focus on our current reality.

The change in deck concept has ruined your solution, I see... But it does not mean you don't have to merge your code and the one that arrived from SVN to make the build working before commiting.

"Encapsulate Quest Opponents in Deck Files" does not mean "push all that stuff into the poor Deck class", it implies limitation only on the way data is stored on disk, not in memory. Moreover bug description may inaccurate, you don't have to follow it literally.

[Max mtg]

And I've already told you that "For now, I'll fix the stuff from r10194, and perhaps this input from Max mtg could be a new issue in the bug tracker."

Only if you manage to set the whole thing up and running.

This sounds a lot like hard-coding the data into a class. That's already in place, in QuestUtil. I thought we were trying to get away from that.

What Exactly you mean by "this"?

Rollback would be fair, at least for the time being while you are fixing the build.

Sure, should I do that, or do you want to? I'll probably screw something up.

ok, I'll do it myself

Anyway, you can just throw out the work if you want, I guess. If it's so horrible then no reason to keep it around.

Unfortunatelly, I'll have to. The work that breaks the build in such a catastrophic way cannot be used for good.

[Doublestrike]

Mate, why are you so pissed off about this? I've done everything to try to help and be flexible, there's no reason to put in little snide remarks. Also, the whole thing was up and running with the previous version of DeckManager. From my perspective, the build was broken already in a catastrophic way.

Thanks for doing that work. I hope others agree with you.

In the future, I'll make smaller changes in my commits, and be sure to update more frequently.

[Max mtg]

r10212 is buildable again. Achieved that by such a command

Code: Select all

svn merge -rHEAD:10193 http://svn.slightlymagic.net/forge/trunk/src

Changes from r10200 by Hellfish were preserved and not rolled back.

Sorry, Doublestrike.

[Doublestrike]

No worries, I learned a lot.

Anyway, I'll just modify quest decks to have the standard [metadata][main][sideboard] structure, and leave the hard coded extras in QuestUtil and quests.txt, then call the issue resolved.

After that, I'll look into the serialization stuff you mentioned.