aelmosalamy / ascii-combat

A simple CLI text adventure game, created for learning purposes, feel free to contribute.
MIT License
69 stars 17 forks source link

Bug in if statement #10

Closed jgrigonis closed 5 years ago

jgrigonis commented 5 years ago

Line 303 in dungeon.py is wrong:

if arg.lower() in (current_room[GROUND] or current_room[SHOP]):

This doesn't work correctly.

It should be more like:

if arg.lower() in current_room[GROUND] or arg.lower() in current_room[SHOP]:

But even that won't work correctly, because sometimes current_room[SHOP] is None.

aelmosalamy commented 5 years ago

Thanks for your concern, I believe the IF statement is correct since if arg.lower() in (current_room[GROUND] or current_room[SHOP]): is not the same as if arg.lower() in current_room[GROUND] or current_room[SHOP] However, you are right there is a big bug here when you try to look at something and the room is empty, I will fix this in next commit. Anything I missed? I appreciate your help

aelmosalamy commented 5 years ago

I fixed it, basically using [] instead of None solves it, the whole problem was because I assigned shop as None when the room got no items to sell, naive mistake, but I fixed it 👍

jgrigonis commented 5 years ago

I think you missed the point here. Please reread what I wrote more carefully.

The reason this doesn't work is because (current_room[GROUND] or current_room[SHOP]) is being evaluated first. The only time this will work correctly is when current_room[GROUND] is falsy, which is []. Any other time, you're not going to get the result you are looking for, here are some examples to explain it more clearly:

x = [1, 2, 3]
y = [4, 5, 6]
z = []

if 6 in (x or y):
    print("Why isn't it there?")

if 6 in (z or y):
    print("Why is it there now?")

Either way, you might luck into this working (most of the time), but the reason it's happening is confusing, and the code is confusingly written.

aelmosalamy commented 5 years ago

I understood it now! Thanks a lot man, Seems like it doesn't work the way I intend, I thought if 6 in (x or y): means If 6 is a part of x or a part of y, but it goes like if x or y evaluate to True, check if 6 in it, Silly me, anyway I will switch it to if arg.lower() in current_room[GROUND] or arg.lower() in current_room[SHOP]: Thanks again mate and sorry for my arrogance 🦄