the-xkcd-community / the-red-spider-project

Coding and xkcd combined, for fun!
http://the-xkcd-community.github.com/the-red-spider-project
Other
40 stars 33 forks source link

Code review on raptor game version 2. #70

Open WesleyAC opened 10 years ago

WesleyAC commented 10 years ago

Hello everyone,

I started work on the second version of my raptor game today.

Here's the idea behind it:

You play in a normal text based adventure game with a huge tree of options. Every time there is an option that isn't complete, you can add something there. The whole thing is backed by a wiki, so it's easy to contribute and rollback spam.

I made it in a couple of hours, but I'm looking for some code review. After that, it'll mostly be story :)

I'm looking for any suggestions, but especally issues with cross-platform compatibility.

Thanks, Wesley.

P.S. If you feel like adding to the wiki, that's great! Just clone https://github.com/WesleyAC/the-red-spider-project.wiki.git

mrhmouse commented 10 years ago

I've opened a branch with code changes starting at mrhmouse/the-red-spider-project@e9a77df3bc1175161c4a638285241a7e20e851a1 for code review. I'll try to keep my commits very small so that it's obvious what I'm changing and why. Feel free to comment on any of the commits if you have improvements over my approach.

I'm not making any major changes to the code such as moving it into a class or adding support for localization; I'm just making minor adjustments.

mrhmouse commented 10 years ago

I've now made major changes to the code (moved the game into a class). However, each commit itself is pretty small, so hopefully my changes will be obvious.

I removed the calls to split for room files, and am instead parsing each file into a Room instance, line by line.

From the room-author's perspective, nothing has changed. From the player's perspective, you now use numbers instead of letters to choose options. It would be trivial to swap back to using letters, if that's preferred.

WesleyAC commented 10 years ago

Looks good, and I can see what's going on. How do I pull your changes backinto my version so that I can edit them? I'm using the command line git, not the github app.

mrhmouse commented 10 years ago

I'm not at my machine right now, but I think you can do this:

    git remote add mrhmouse https://github.com/mrhmouse/the-red-spider-project.git

    git fetch mrhmouse

    # on your branch
    git pull mrhmouse raptor-code-review

Sorry if the formatting is off, I'm on my phone.

jgonggrijp commented 10 years ago

It works totally fine on OS X, except for the problem with FileNotFoundError (which somehow keeps reappearing in your code!) but I just pushed a fix for that and sent you a pull request. So basically it just works totally fine on OS X.

Some random remarks:

WesleyAC commented 10 years ago

@jgonggrijp To point 1: Sounds good. I've been busy lately what with school all week and being involved in FIRST on the weekends, but I should be ready soon. I want to add some sort of if statement to the wiki so that you can have, like if key then opendoor else fuckoff end (psudocode) 2: Good idea, I'll try to do that soon [done] 3: Well, the wiki could have changed resulting in them having different options, etc, but I'll probably remove it since it's unlikely. [done]

jgonggrijp commented 10 years ago

I have now also tested the raptors game (latest commit) on Windows 7, and with a minor tweak it works like a charm. Without the tweak, I got this:

You do not have a copy of the raptor game data files. Do you want to download them? (y/N)
? y
Downloading raptor game data, please wait...
Traceback (most recent call last):
  File "C:\Users\Julian\Documents\RedSpider\src\raptors.py", line 273, in <module>
    game = RaptorGame(game_dir)
  File "C:\Users\Julian\Documents\RedSpider\src\raptors.py", line 115, in __init__
    self.setup()
  File "C:\Users\Julian\Documents\RedSpider\src\raptors.py", line 188, in setup
    self.clone(False)
  File "C:\Users\Julian\Documents\RedSpider\src\raptors.py", line 143, in clone
    subprocess.call(self.__git_args)
  File "C:\Python27\lib\subprocess.py", line 493, in call
    return Popen(*popenargs, **kwargs).wait()
  File "C:\Python27\lib\subprocess.py", line 679, in __init__
    errread, errwrite)
  File "C:\Python27\lib\subprocess.py", line 896, in _execute_child
    startupinfo)
WindowsError: [Error 2] The system cannot find the file specified

The reason for that crash is that subprocess.call needs shell=True on Windows whenever you call a program that must be searched in PATH. So the solution is to add shell=True. I'll mark this on the relevant lines in the diff.