jmoon018 / PacVim

GNU Lesser General Public License v3.0
3.25k stars 193 forks source link

isFullDigits as regex #13

Closed manonthemat closed 9 years ago

jmoon018 commented 9 years ago

Hmm, I am having problems with this version. The game loads, I start the level, the Ghosts move, but immediately after pressing any key (q, w, h, etc), there is an error: "terminate called after throwing an instance of 'std::regex_error' what(): regex error Aborted (core dumped)"

I am using Ubuntu (if that makes any difference). I'd be happy to merge after it works for Ubuntu

manonthemat commented 9 years ago

What version of g++ are you using?

jmoon018 commented 9 years ago

version 4.8.2

adrian17 commented 9 years ago

According to this, <regex> is properly supported from 4.9, unfortunately.

jmoon018 commented 9 years ago

Thanks @adrian17!

Since this is the case, I don't think it would be wise to proceed with regex (unless it can magically work with versions 4.8.X). I want to keep the game as accessible as possible, and saving a few lines of code is probably not worth the risk spawning new playability issues.

I know that I did ask for regex in the source code comments, so I do appreciate the help, but I apologize because I have to backtrack on this. Thanks, @manonthemat

manonthemat commented 9 years ago

Understood. In that case you also might want to adjust your makefile as it's specifically asking for g++, which might not be installed on the compiling machine. The use of $(CXX) in the Makefile might be the platform independent answer to that.

manonthemat commented 9 years ago

The most recent commit in this pull request compiles isFullDigits the old way if g++ prior 4.9 is used to compile the code.

jmoon018 commented 9 years ago

@manonthemat

Makefile: I receive these errors in Ubuntu after calling make:

g++ src/*.cpp -pthread -std=c++11 -stdlib=libc++ -lncurses -o pacvim
g++: error: unrecognized command line option ‘-stdlib=libc++’
make: *** [all] Error 1

It works great on OS X though -- no errors.

isFullDigits: Your changes are pretty cool, and they work for both OS X/Ubuntu, but I do have some reservations about merging them. I highly prefer code that is more simple and/or concise; the reason I asked for Regex in the comments, which I apologize for not articulating earlier, was because I thought it would be an easy change w/ leaner code. I'd most likely accept extra lines code only if it produces something new (bug fixes, features, etc). In this case, the isFullDigits method currently works, so the only changes I would allow are ones that reduced the code (which I thought Regex would do). I apologize for not clarifying, I hope you understand

jmoon018 commented 9 years ago

Closing this request. Adding regex is more trouble than it's worth IMO