MapleShrine/mkxp
1
0
Fork 0
Code Issues 32 Pull requests 11 Projects Releases Wiki Activity

Set debug variables from argv #191

Merged
ReinUsesLisp merged 2 commits from master into master 2018-02-22 08:08:46 +00:00
Conversation 8 Commits 2 Files changed 5 +46 -1
ReinUsesLisp commented 2018-02-18 22:45:46 +00:00 (Migrated from github.com)
Copy link

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

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].
binding-mri/binding-mri.cpp
Ancurio (Migrated from github.com) commented 2018-02-21 20:54:18 +00:00
Copy link

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

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

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

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`
binding-mruby/binding-mruby.cpp
Ancurio (Migrated from github.com) commented 2018-02-21 20:58:18 +00:00
Copy link

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.
src/filesystem.cpp
Ancurio (Migrated from github.com) commented 2018-02-21 20:58:48 +00:00
Copy link

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

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

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

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:
binding-mruby/binding-mruby.cpp
@ -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
Copy link

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

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

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

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

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.
Reviewers
No reviewers
Ancurio
Labels
Clear labels   
RGSS accuracy

   
bug

   
compilation

   
discussion

   
documentation

   
duplicate

   
enhancement

   
invalid

   
performance issue

   
port request

   
question

   
ruby incompatibility

   
wontfix
No labels
RGSS accuracy
bug
compilation
discussion
documentation
duplicate
enhancement
invalid
performance issue
port request
question
ruby incompatibility
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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.
Delete branch "master"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?