Page MenuHome

blenderplayer - Reviewed some command line options and the respective help
ClosedPublic

Authored by Inês Almeida (brita_) on Feb 12 2014, 11:51 PM.

Details

Summary

Doubts: are written as comments in the code
Current Problems:

  • stereo modes sometimes have different names and are ordered differently everywhere (blender properties panel names and tooltips, blenderplayer cmndline)
  • dome mode as several options, but can only take one at a time, like -D mode bla -D tilt X -D ...
  • gameoptions are also given one at a time, like above, but with an assignment -g option = X . This is inconsistent.
  • gameoptions need to be reviewed and docummented
  • parent to window (-i) is useful for what? (there is no wiki on this either)
  • there is no support for verbose options like --help (blender itself supports it)

Diff Detail

Event Timeline

The parent window option is used for embedded. In general, our argument parsing just needs to be re-written (possibly making use of the system that Blender uses). That being said, cleaning up what we have isn't bad, and overall I like the changes.

source/gameengine/GamePlayer/ghost/GPG_ghost.cpp
198

We should probably leave the space out here and let the printf string handle the space.

225

This should probably be indented.

232

I think bits per pixel and frequency both default to the values in their blend files.

576

I don't see any change here, other than some whitespace. Whitespace changes should probably be left to their own patch unless you're specifically fixing whitespace.

730

It looks like you're already adding "new" errors for the window and fullscreen parameters, so we might as well add this one too. Old scripts should be simple to update (and I don't think there are many that would require an update because of this).

Bah, that's what I get for posting at 3am. The parent window ID is useful for people embedding the Blenderplayer into another application (e.g., Burster: http://geta3d.com/).

These are the inline comments I made when I submitted (I did not realize they were not immediately public).

source/gameengine/GamePlayer/ghost/GPG_ghost.cpp
580–582

I have added blocks { } to the cases and the comments in the same line so that the switch can be folded in QtCreator for easy viewing

649

removed this because 'no stereo' is already set as default int the declaration of 'stereomode'

And these are in reply

source/gameengine/GamePlayer/ghost/GPG_ghost.cpp
198

I did not touch the space, I only added '[ ]' because -c is optional

225

224 has no \n, so 224+225 shows up as one indented line, but it was to big to fit in the code.

232

Everything defaults to the blend file. I wrote this in 216.
What I meant is that, if it was not changed in the blend file, then.. that is the default.
Something like line 236 is more clear?

576

As I added blocks to the cases, and this one already had a block, I pulled this block one tab left as it affects less lines than pulling all the other cases one tab right. I don't know why it only registered the indentation change in this line. Because it is a comment, maybe?

Inês Almeida (brita_) updated this revision to Unknown Object (????).Feb 16 2014, 12:14 PM
  • blenderplayer options review - corrections

This looks good to me, but we'll have to wait until bcon1 be it gets committed. Feel free to remind me if I forget.

Checked that this patch is compatible with the current master. @Mitchell Stokes (moguri) can this go in now? We are at BCon1