Set debug variables from argv #191

Merged
ReinUsesLisp merged 2 commits from master into master 2018-02-22 08:08:46 +00:00
ReinUsesLisp commented 2018-02-18 22:45:46 +00:00 (Migrated from github.com)

Propietary RGSS editors call Game.exe with debug, test or btest arguments (according to issue #190), setting $DEBUG, $TEST or $BTEST respectively as true.

To use it replace Game.exe with the mkxp binary.
Then when using some propietary editor from wine, mkxp will be called instead of a propietary implementation and named flags are included.

Propietary RGSS editors call Game.exe with debug, test or btest arguments (according to issue #190), setting $DEBUG, $TEST or $BTEST respectively as true. To use it replace Game.exe with the mkxp binary. Then when using some propietary editor from wine, mkxp will be called instead of a propietary implementation and named flags are included.
Ancurio (Migrated from github.com) requested changes 2018-02-21 21:01:53 +00:00
Ancurio (Migrated from github.com) left a comment

I would put the parsed bool triplet into the Config struct , and do the parsing alongside the normal (boost) command line parsing.
Also might make sense to only have boost parse the command line if none of debug, test or btest are at argv[1].

I would put the parsed bool triplet into the Config struct , and do the parsing alongside the normal (boost) command line parsing. Also might make sense to only have boost parse the command line if none of debug, test or btest are at argv[1].
Ancurio (Migrated from github.com) commented 2018-02-21 20:54:18 +00:00

The block above is not specific to MRI, it doesn't belong in the binding

The block above is not specific to MRI, it doesn't belong in the binding
Ancurio (Migrated from github.com) commented 2018-02-21 20:55:16 +00:00

I don't believe I have used snake_case anywhere in mkxp

I don't believe I have used snake_case anywhere in mkxp
Ancurio (Migrated from github.com) commented 2018-02-21 20:56:26 +00:00

Please use variable == "constant" order; we don't need defensive coding when compilers do a good enough job of catching our errors these days.

Please use `variable == "constant"` order; we don't need defensive coding when compilers do a good enough job of catching our errors these days.
Ancurio (Migrated from github.com) commented 2018-02-21 20:57:53 +00:00

There is a convenience function for C++ → Ruby bool conversion in binding-util.h

There is a convenience function for C++ → Ruby bool conversion in `binding-util.h`
Ancurio (Migrated from github.com) commented 2018-02-21 20:58:18 +00:00

I think all comments on the MRI binding code apply here as well.

I think all comments on the MRI binding code apply here as well.
Ancurio (Migrated from github.com) commented 2018-02-21 20:58:48 +00:00

Looks like a commit from master sneaked in

Looks like a commit from master sneaked in
ReinUsesLisp commented 2018-02-22 05:25:41 +00:00 (Migrated from github.com)

Sorry for those commits.
Instead of stop boost-parsing when debug, test or btest are found, argv is moved one argument, to allow something like: $ ./mkxp debug --fullscreen true
RGSS1 uses $DEBUG and VX uses $TEST ($BTEST is used by both), so not only style was wrong.

I'm writing a free alternative to known privative XP editor, that's why I'm interested in these calls.

Sorry for those commits. Instead of stop boost-parsing when debug, test or btest are found, argv is moved one argument, to allow something like: `$ ./mkxp debug --fullscreen true` RGSS1 uses $DEBUG and VX uses $TEST ($BTEST is used by both), so not only style was wrong. I'm writing a free alternative to known privative XP editor, that's why I'm interested in these calls.
Ancurio commented 2018-02-22 06:38:15 +00:00 (Migrated from github.com)

No need to apologize, your commits look very good now! Thanks for working on this. :)
You should combine all the changes you made so far into one commit, you can do that with
git rebase -i master and then marking all commits before your newest one as fixup or f. Then do a
git push --force ... to update your branch.
This makes the commit history look clean, and easier for me to review.

No need to apologize, your commits look very good now! Thanks for working on this. :) You should combine all the changes you made so far into one commit, you can do that with `git rebase -i master` and then marking all commits before your newest one as `fixup` or `f`. Then do a `git push --force ...` to update your branch. This makes the commit history look clean, and easier for me to review.
Ancurio (Migrated from github.com) approved these changes 2018-02-22 06:44:06 +00:00
Ancurio (Migrated from github.com) left a comment

Except for the one comment in the mruby binding, looks very good now. Combine all your changes into one commit with a proper message, and it's good to merge. 👍

Except for the one comment in the mruby binding, looks very good now. Combine all your changes into one commit with a proper message, and it's good to merge. :+1:
@ -114,8 +114,17 @@ static void mrbBindingInit(mrb_state *mrb)
/* Load RPG module */
Ancurio (Migrated from github.com) commented 2018-02-22 06:41:06 +00:00

Might want to set TEST here too according to rgssVer like you did in MRI, just for consistency.

Might want to set `TEST` here too according to `rgssVer` like you did in MRI, just for consistency.
ReinUsesLisp commented 2018-02-22 07:40:10 +00:00 (Migrated from github.com)

git is kind of tricky sometimes. I used git reset --soft ... in the end, it took me some time to move away the filesystem commit (it was between commits).

git is kind of tricky sometimes. I used `git reset --soft ...` in the end, it took me some time to move away the filesystem commit (it was between commits).
Ancurio commented 2018-02-22 08:08:31 +00:00 (Migrated from github.com)

Looks good to go; I'm just confused why your commit b022242 is still there. It looks like your master branch is based off of an out-of-date master (so why does github tell me "no conflicts when rebasing"?). I think I'll just pull this to see what happens, and correct my master afterwards manually.

Thanks!

Looks good to go; I'm just confused why your commit b022242 is still there. It looks like your master branch is based off of an out-of-date master (so why does github tell me "no conflicts when rebasing"?). I think I'll just pull this to see what happens, and correct my master afterwards manually. Thanks!
Ancurio commented 2018-02-22 08:10:47 +00:00 (Migrated from github.com)

Turns out, the overfluous commit just disappeared :) Nice

Turns out, the overfluous commit just disappeared :) Nice
ReinUsesLisp commented 2018-02-22 08:14:12 +00:00 (Migrated from github.com)

I manually re-commited filesystem's commit into b022242 after the "squash". Maybe that messed it up.

I manually re-commited filesystem's commit into b022242 after the "squash". Maybe that messed it up.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: MapleShrine/mkxp#191
No description provided.