search

comp345 / risk-game

Warzone game on console implemented using C++
1 stars 0 forks source link

Blockade Order, Negotiate, testing in Driver #22

Closed laila-chammaa closed 2 years ago

laila-chammaa commented 2 years ago

got blockade order working and tested tested bomb order started on negotiate, but would be better to merge this and open another pr for that? fixed some todos for adjacency checking

eyeshield2110 commented 2 years ago

Btw, i dont think we should merge the branch into a2_archived, we should leave a2_archived as our last point of reference from a2. When the implementation of Blockade is approve, let's just close this PR without merge. @laila-chammaa

laila-chammaa commented 2 years ago

@eyeshield2110 why close? ee can merge into a3 branch or smth

eyeshield2110 commented 2 years ago

@laila-chammaa Is it possible to change the target branch of a PR?

laila-chammaa commented 2 years ago

@laila-chammaa Is it possible to change the target branch of a PR?

yep, to where

eyeshield2110 commented 2 years ago

@laila-chammaa Is it possible to change the target branch of a PR?

yep, to where

No need to merge to any a3 branch yet, so none for now

eyeshield2110 commented 2 years ago

image Here, in the execute() of Blockade You're removing all the territories from the player executing the Blockade and giving them to the Neutral player? The Blockade should just reassign the target territory to the neutral player, no?

laila-chammaa commented 2 years ago

omg you're right, nice catch edit: actually no, the comment is incorrect but code is fine, i search to find the target, then remove it from the player's territories and add it to the neutral player. lemme fix the comment

laila-chammaa commented 2 years ago

@eyeshield2110 also, i'd prefer to merge, don't want to have a billion branches to merge at the last minute again, let's merge consistently. if u have any changes, open up a pr and let's merge to a3 branch. even merging to a2 is fine, no need to actually pinpoint what we submitted to a2 was, what's the point?

eyeshield2110 commented 2 years ago

@laila-chammaa i am finding a lot of bugs across every parts, having untouched,"archived" branches makes tracking functioning version of the code easier than trying to find a particular commit in the commit logs

eyeshield2110 commented 2 years ago

We can merge your branch into a3_fixe (dont remember the name of the branch) when the bugs are done being fixed (which are not yet)

laila-chammaa commented 2 years ago

We can merge your branch into a3_fixe (dont remember the name of the branch) when the bugs are done being fixed (which are not yet)

fine but even a2_archived isn't "functioning" code at this point. let me know when it's good to merge

eyeshield2110 commented 2 years ago

We can merge your branch into a3_fixe (dont remember the name of the branch) when the bugs are done being fixed (which are not yet)

fine but even a2_archived isn't "functioning" code at this point. let me know when it's good to merge

Either way, let's not merge into a2_archived, but into my branch a3_fix_p3 It's not like we'll accidentally merge a2_archived.

eyeshield2110 commented 2 years ago

omg you're right, nice catch edit: actually no, the comment is incorrect but code is fine, i search to find the target, then remove it from the player's territories and add it to the neutral player. lemme fix the comment

Ah ok, i get it now, thanks for clarifying.

eyeshield2110 commented 2 years ago

Finished the Negotiating order. The constructors, helper functions, getter and setters should work. Negotiate is tested (valid, invalid) and Advance order is tested to see if it leads to attack (should not) between two negotiating players.

The method removeAllNegotiation() should be call in GameEngine at the end of ExecuteOrders phase to reset the negotiation vectors.

We should be able to merge this branch into a3_fix_p3, where I can fix the cards.

eyeshield2110 commented 2 years ago

Merge done. Delete branch when review done @laila-chammaa