Issue #34

[benmrtnz27]

Hey I hope this is what you were looking for! Added dialogue for when the player enters a shop that creates them. Also finished adding Type Hints to some of the files if that's okay. Thanks a lot, this was a very fun project to work on!

[Grimmys]

Hello, It seems rather good for me, I will test it soon but at least the CI (continuous integration, i.e. unit tests) is passing and I don't see any blocker in the implementation you introduced.

However, I'm a bit puzzled with the type hints you replaced.

Why did you decide to replace all the "tuple[]/list[] etc." by their uppercase equivalent (i.e "Tuple[]/List[] etc.")? I prefer the lowercase version because they don't require any import, and I think it's the advised way to do it since Python 3.9. See this: https://stackoverflow.com/a/62033243

[benmrtnz27]

Hello! Thanks for taking a look at it! About the type hints, I'm using Pycharm and the IDE was giving me errors with the message to replace the type hints with the uppercase variants. If you'd like I can go back and change them back?

[sethstenzel]

@benmrtnz27 changing them back to what they were would be correct. Are you sure you loaded your virtual env with the right version of python for the game which I think is 3.9.6?

In pycharm if I load with my 3.9.6 env, the lowercase type hints are fine, but if I switch back to an older env like what I use for work 3.6.4 then pycharm warns me that tuple cannot be parameterized directly

[Grimmys]

Yes you have some errors probably because you are not on Python >=3.9, you can revert them it should be working. Maybe it should be stated somewhere that Python >=3.9 is required.

If it's too hard for you to undo the changes or you don't have time it's ok I can merge as it is. 😄

[CoolCat467]

Ok so I read everything, and it appears that the only files you need to merge are the following: maps/level_0/data.xml src/game_entities/shop.py src/services/load_from_xml_manager.py The rest are changing type annotations only. src/game_entities/shop.py has type annotations, but it also changes slight things about interacting with the shopkeeper

[Grimmys]

Yes, thank you for the review, but still I can't select which files I want to merge, that is not how a PR is working.

The unwanted changes should be reverted on the benmrtnz27:master branch, I will check if I have a write access on it to do it. 👍

[Grimmys]

Ok great, I had the right to push the revert. 👍

So I look at the feature in the game and it is working fine, thank you for your contribution @benmrtnz27!