powergrid-comp-345 / PowerGrid

0 stars 2 forks source link

Questions regarding potentially changing how resource tokens are handled (and related stuff) #3

Open denizakcal opened 5 years ago

denizakcal commented 5 years ago

This is the message I had posted on Facebook (with slight modifications).:

"*I've been looking at the non-graph code (since it's related to the object generation that follows the xml processing), and I feel that inheriting from GamePiece does not help with code reuse (which is the point of inheritance) and instead just makes things unnecessarily more complicated. Also, if one needs to specify which type of game piece an object that inherits from GamePiece is (as is the case with the methods in the GamePiece class), why not just have classes that are unrelated (in terms of inheritance)?

*It is unclear to me whether the ResourceToken class is supposed to keep track of one specific resource token or all of them (but I also think that that's a moot point as described in the next bullet point).

*I think it would be better if the Player class just had an std::vector of powerPlants (which it already does), where the PowerPlants should keep track of how many resource tokens of each type (oil, coal, uranium, garbage) that they have (which they already do). If there's ever a need for the player to know how many of a certain token he or she has, it can be found by counting the aforementioned player's power plants' tokens (but I don't think that that will be needed).

Do you guys agree (regarding the first and third of the three bullet points above)?

*The game's resource (token) supply could be determined with the following (pseudo)code at the beginning of each of the appropriate phase or whatever (I need to look at the rules of the game again). [Edit: I'm keeping the original pseudocode, directly below, for reference in this original post, but it's incorrect; the pseudocode in the second comment in this issue page should be the correct version.]:

const int TOTAL_COAL = 24; const int TOTAL_OIL = 24; const int TOTAL_GARBAGE = 24; const int TOTAL_URANIUM = 12;
int usedCoal = 0; int usedGarbage = 0; int usedUranium = 0; int usedOil = 0;
for each Player player
    for each PowerPlant powerPlant in player
        for each coal in powerPlant
            usedCoal++;
        for each oil in powerPlant
            usedOil++;
        for each coal in powerPlant
            usedUranium++;
        for each garbage in powerPlant
            usedGarbage++;

int coalSupply = TOTAL_COAL - usedCoal; int oilSupply = TOTAL_OIL - usedOil; int garbageSupply = TOTAL_GARBAGE - usedGarbage; int totalUranium = TOTAL_URANIUM - usedUranium;

*According to the [game's] rules, a power plant can store twice as many resources as it needs, but a player CANNOT take the resources that one his or her power plants has and put them on one of the other power plants he or she owns, right? (That shouldn't change the design much, but I'm curious since it does technically matter.) (Also, I think that the PowerPlant class should enforce a maximum amount of resource tokens [(which can vary from one power plant card to another)], which it doesn't seem to currently do.)

Let me know if I'm misunderstanding anything.

P.S. Sorry for the delay (regarding the GameState object part of getting all our parts working together properly), but I need to be clear on the structure of all objects before being able to have a GameState object that works as it should. (Also, I'm sick, so my performance is a little worse.)"

denizakcal commented 5 years ago

About the above pseudocode, I found a flaw. Here's the improved version.:

const int TOTAL_COAL = 24; const int TOTAL_OIL = 24; const int TOTAL_GARBAGE = 24; const int TOTAL_URANIUM = 12;
int usedCoal = 0; int usedGarbage = 0; int usedUranium = 0; int usedOil = 0;
for each Player player
    for each PowerPlant powerPlant in player
        usedCoal += powerPlant.getAmountOfCoalTokensPlacedOnTheCard();
        usedOil += powerPlant.getAmountOfOilTokensPlacedOnTheCard();
        usedGarbage += powerPlant.getAmountOfGarbageTokensPlacedOnTheCard();
        usedUranium += powerPlant.getAmountOfUraniumTokensPlacedOnTheCard();

int coalSupply = TOTAL_COAL - usedCoal; int oilSupply = TOTAL_OIL - usedOil; int garbageSupply = TOTAL_GARBAGE - usedGarbage; int totalUranium = TOTAL_URANIUM - usedUranium;
icharette commented 5 years ago

Hey again here, the ressourcetoken class was meant to only keep track of one type at a time. I will also look at this tomorrow and discuss it with you.

icharette commented 5 years ago

we asked Nora and she said we should be able to move ressources from one power plant to another, like when we played the game in person.

I hear you on the issue of inheritance, I agree.

icharette commented 5 years ago

did you push your most recent commits? is the version online the most updated version? If so, I will pull and make the modifications.

denizakcal commented 5 years ago

My almost-most-recent changes are in the UpstreamGraphFixingBranch. And, about the moving resource tokens from one plant to another, alright. :)

denizakcal commented 5 years ago

About the inheritance, okay, good, so I didn't misunderstand anything. :)

Also, I'm not sure if I wrote this or just thought it (:P), but wouldn't it be better for PowerPlant to keep track of resource tokens as int fields?