-
Notifications
You must be signed in to change notification settings - Fork 1
Add files via upload #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Last attempt at Pull Request Failed, fixed the critical error
hitz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll run some tests and fix it up maybe... I dunno when.
| self.tapped = False | ||
| if self.cipt: | ||
| self.tapped = True | ||
| if context is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably we should remove this case, but it breaks a test or script.
| import random | ||
| import json | ||
| import mtginator.cards as cards | ||
| import cards |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do the tests still work with this change?
| if len(lands) > 0: | ||
| pick = random.choice(lands) | ||
| # Note: should implment some color optimization | ||
| pick.play(None) #lands don't need context to play |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh here you have a null context, probably this should assume a board state/context, even if it's irrelvant for land drops
| def enumerate_plays(self): | ||
| ''' For a given hand (or metahand) and available mana, what plays are available to Player | ||
| Returns: Set of Card objects | ||
| Returns: List of Card objects |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I made this a Set() on purpose, like what if you have Forest, Forest, Forest, Llnaowar Elves? You don't need an array of 4. But possibly i overlooked something.
| #!/usr/bin/env python | ||
|
|
||
| import sys | ||
| sys.path.insert(0,'/src/mtginator/') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok this is horrible and we need to fix imports if that's what you need.
|
Ben,
I believe I have installed bootstrap, but I still can't get the project to
make -- so far on this first pass I have been just calling sim.py in the
command line.
When I type:
buildout bootstrap
I get an error message "buildout command not found", is buildout something
that needs to be installed?
Thanks for your patience with me. :)
Once I can properly make the project, I should be able to crush some bugs,
and make sure that it passes the tests.
There is no hurry on this, I want to respect your time.
…On Tue, Aug 7, 2018 at 3:36 PM, Ben Hitz ***@***.***> wrote:
***@***.**** commented on this pull request.
I'll run some tests and fix it up maybe... I dunno when.
------------------------------
In src/mtginator/cards.py
<#12 (comment)>:
> @@ -230,11 +330,18 @@ def pay_cost(self, context):
def play(self, context):
- self.pay_cost(context) # need some sort of game context object
- self.zone = 'battlefield'
- self.tapped = False
- if self.cipt:
- self.tapped = True
+ if context is None:
probably we should remove this case, but it breaks a test or script.
------------------------------
In src/mtginator/decks.py
<#12 (comment)>:
> import random
import json
-import mtginator.cards as cards
+import cards
do the tests still work with this change?
------------------------------
In src/mtginator/game.py
<#12 (comment)>:
> @@ -92,28 +106,39 @@ def main(self):
self.make_play(plays)
def land_drop(self):
- lands = [c for c in self.hand if c.is_land]
- pick = random.choice(lands)
- # Note: should implment some color optimization
- pick.play()
- print("Played {} as Land for turn [ hand size: {} ]".format(pick, len(self.hand)))
+ lands = [c for c in self.hand if c.is_land()]
+ if len(lands) > 0:
+ pick = random.choice(lands)
+ # Note: should implment some color optimization
+ pick.play(None) #lands don't need context to play
oh here you have a null context, probably this should assume a board
state/context, even if it's irrelvant for land drops
------------------------------
In src/mtginator/game.py
<#12 (comment)>:
> @@ -144,16 +169,20 @@ def mulligan(self, rules=None, n=7, verbose=True):
def enumerate_plays(self):
''' For a given hand (or metahand) and available mana, what plays are available to Player
- Returns: Set of Card objects
+ Returns: List of Card objects
I think I made this a Set() on purpose, like what if you have Forest,
Forest, Forest, Llnaowar Elves? You don't need an array of 4. But possibly
i overlooked something.
------------------------------
In src/mtginator/sim.py
<#12 (comment)>:
> @@ -1,11 +1,13 @@
#!/usr/bin/env python
import sys
+sys.path.insert(0,'/src/mtginator/')
ok this is horrible and we need to fix imports if that's what you need.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#12 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AmpeH6PCGRg-ruZqg_v0x2sqpIUubv-Tks5uOhZbgaJpZM4Vxd5h>
.
|
Removed the critical error from last pull request attempt.
This version counts the players mana by counting untapped basic lands.
One random spell that the player can cast is cast each turn (it taps lands)
Cards played are removed from the player's hand (land drops and spells cast)