inland dragon
https://paste.pythondiscord.com/VDFA
Newbie here seeking code review and feedback. Is my code good? bad? What can be improved? Any best practices I should be implementing
Gameplay loop:
wintry locustBOT
https://paste.pythondiscord.com/VDFA
Newbie here seeking code review and feedba...
@inland dragon
Python help channel opened
inland dragon
@main siren you available?
main siren
you misspelled prestiege (=> prestige)
is this working fine now?
line 113 – under what conditions will that code be executed?
inland dragon
it's only used to get out of the visit_store() and back to Game.continue_game(cls)
inland dragon
is this working fine now?
Yes, seems to be ok
main siren
can you answer my question though?
main siren
sorry, I misread the code and jumped to the wrong conclusion; ignore me.
it's fine.
the Game class looks odd and shouldn't be there
this is not how classes are supposed to be used at all
inland dragon
I'm going to be honest though. Most of the time, I have no idea whether I'm linking things up correctly. Like when to use cls or self.
inland dragon
this is not how classes are supposed to be used at all
Ok, please elaborate on this. how should they be used?
main siren
a class is supposed to be a structured template for an object
it represents some concept
let's talk about Items
what does an Items instance represent?
inland dragon
A new item like: Sword, Shield, Potion
main siren
so if you have three instances of Items, what do you call a bunch of them? Itemses?
you yourself used a singular form when describing one such thing
inland dragon
I don't get it. Aren't they all instances of class Items?
main siren
is a Sword an Items then?
inland dragon
Yeah
main siren
so if you have a Sword and a Shield, what do you call that collection? Itemses?
inland dragon
Items no?
main siren
much like you have str and int classes, they're not called strings or integers
the singular form reflects the fact that 1 or "abc" is a single integer or string
inland dragon
so the class Items should be renamed to Item?
main siren
yes, for starters
what's also odd to me is that there is a class method in Items that constructs all the items
isn't that something the game itself should be doing?
why does the Items class have to know about every possible item?
inland dragon
I figured that since its related to Items, I should just put it all in the same place.
And if more items were to be added in the future, it would have cluttered the Game class.
main siren
isn't the initial collection of items inherent to the logic of the game though?
that clutter always goes somewhere, and I don't think the Item class should care about that
like the others pointed out yesterday, this way of using classes as namespaces is highly unorthodox
for example you can't have two different games with different parameters this way
inland dragon
Sorry, I don't understand.
Game class was odd and shouldn't be there
main siren
no, because they're both modifying the same Player and Items classes
so you can't have two truly separate Game instances this way
the normal way would be to have the Game instance instantiate a Player object and initialize the list of Items within that game
imagine what would happen if you were to play another game after the old one ended
inland dragon
Ok, so I should:
Game, andPlayer an instance variable by adding a def __init__?
inland dragon
imagine what would happen if you were to play another game after the old one end...
The game program would close, and on opening it in the next session, it would reset back to default, no?
main siren
I mean, if you tried to run the game again without closing the interpreter
sure, if you closed the program entirely, it would reset back to defaults
but you asked for best practices
inland dragon
Ok, in that case, the same data would carry over to the new Player?
main siren
if you did not make an instance of it, yes
another thing is that it's a faux pas to wait for user input in __init__()
__init__() should never block for any reason
I think a Game.run() method would be reasonable
inland dragon
Ok, so in this case the Game __init__() should only have the name property? Everything else like create player and initalize items should go under a run() method?
main siren
what's the name property about?
inland dragon
Something like this? so new instance has a name.
class Game:
def __init__(self, name):
name = name
what are you using the name property for?
inland dragon
Nothing lol. Just thought it should have a name
main siren
don't add useless things to it
I think the Game.run() method should initialize the game and then have a loop where it asks for the player's action, and then executes those actions
the way you're doing it now, if I'm not mistaken, is that you call Game.continue_game() before returning from the previous call
this is bad because it's basically a recursive call, adding one stack frame for every new action
see what you can come up with when implementing game.run()
inland dragon
this is bad because it's basically a recursive call, adding one stack frame for ...
What does this mean? Please dumb down
main siren
when you call a function, you should also return from that function in a timely manner
because the interpreter keeps track of these calls
if something goes wrong, you get a "traceback", right?
where you see the exception raised, and all the function calls in your current stack, starting from Game.__init__() (as things stand right now)
inland dragon
see what you can come up with when implementing `game.run()`
Will it be something like this?
class Game:
def __init__(self):
self.game_run()
def game_run(self):
# Initialize players, items, etc
I think the game_ prefix is redundant
and also don't call game_run() in the initializer
call it on a Game instance you create in the main body of the script
main reef
Yeah, what we want seems to be some sort of while loop where the game repeatedly asks you for an action
main siren
like Game().run()
main reef
Yeah, what we want seems to be some sort of `while` loop where the game repeated...
unless for some reason the game needs to be start and stopped by some external controller - then you might want to put the while loop in that external controller
inland dragon
where you see the exception raised, and all the function calls in your current s...
What's a traceback? Does it mean that if functions don't have a return statement, it keeps on accumulating stuff until it slows to a crawl?
So to avoid this, best practice is to always have a return statement?
main siren
doesn't need a return statement necessarily, so long as you exit from the function instead of calling itself again
you must have seen a traceback when your program crashed
inland dragon
and also don't call `game_run()` in the initializer
Ok, does this mean that my def __init__(self) will be empty? like this?
class Game:
def __init__(self):
# Nothing here
def run(self):
# Initialize things here
there's no point in having an empty __init__()
an __init__() method is not necessary for instantiating a class
that said...
you would probably want to initialize some member variables there
depending on the structure your Game class ends up with
if you ever add any parameters to your game, those would be saved in __init__ ()
inland dragon
doesn't need a `return` statement necessarily, so long as you exit from the func...
Ok, how do I know if I've successfully exited a function and that there is no accumulated traceback inducing stuff?
main siren
if you're not calling the function from itself, or calling another one that calls the same function again
there are ways to programmatically check for this, but it's not normally done
inland dragon
if you ever add any parameters to your game, those would be saved in `__init__ (...
Ok, what kind of parameters would a Game class have? There is only one 'game' going on, no?
main siren
well, maybe like "difficulty" or "starting money", etc.
inland dragon
the way you're doing it now, if I'm not mistaken, is that you call `Game.continu...
Sorry, which previous call isn't being returned? I think I understand, but unsure where the issue is coming from. What do I look for in the code?
main siren
line 134 in your paste
inland dragon
So if it was return Game.continue_game(cls), there would not be any traceback errors?
main siren
no, you're missing the point
if you're calling a Game.continue_game() which then causes another call to Game.continue_game() before returning from the previous call, you end up with a perpetually growing stack
this is why your game should have a top-level while loop for processing player actions
inland dragon
Ok, but how is it not returning from the previous call?
main siren
because it's unconditionally calling Game.continue_game() before it can do so
see line 66
inland dragon
I don't get it. I mean, I get what you're saying, but I don't see it from the code. If possible, please down dumb a few more levels lol.
Like continue_game(self), I go to store, then I purchase an item, then I hit 'q' and it goes back to continue_game(self)?
main siren
you are calling that function within the while loop
it doesn't go back to the same call of continue_game(), but it starts a new one!
and that is the problem
inland dragon
Hmm, but it is outside the while loop, no? It has the same indentation. As in after exiting the loop, it calls continue_game()
And, how would I make it go back to the same call?
Ok, I need to step out for a bit. Thanks for the help again @main siren !. Will refactor seek review again
main siren
line 66 is indented on the same level as the match
so it's inside the while loop
just make a top-level interaction loop, and don't call that method from any other method
wintry locustBOT
Python help channel closed