Skip to content
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

Code review on raptor game version 2. #70

Open
WesleyAC opened this issue Jan 3, 2014 · 7 comments
Open

Code review on raptor game version 2. #70

WesleyAC opened this issue Jan 3, 2014 · 7 comments
Assignees
Labels

Comments

@WesleyAC
Copy link
Contributor

WesleyAC commented Jan 3, 2014

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
Copy link
Contributor

I've opened a branch with code changes starting at mrhmouse/the-red-spider-project@e9a77df 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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Member

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:

  • I would recommend working towards a pull request at this point (before doing more grand improvements). Your program is already pretty good, it just needs more content and I think it will be easier to get more content if more people play it. (But if you think otherwise just ignore this point.)
  • If you are just updating the game data files, you can also do a git pull instead of deleting the old version first and then doing a fresh git clone.
  • Why exactly are you warning players that updating the game files may result in an unplayable game?

@WesleyAC
Copy link
Contributor Author

@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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants