diego1996 / gamekit

Automatically exported from code.google.com/p/gamekit
0 stars 0 forks source link

OgreKitStartup.cfg path not flexible. #72

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
OgreKit load his config file only if it is named OgreKitStartup.cfg and placed 
in the working directory. This is causing me some issue while writing the 
blender plugin:
-I save the current working blendfile as /tmp/gktmp.blend (configurable)
-I execute gamekit against this saved .blend (working directory is configurable)
-I want to create a gamekit config file based on the blender parameters 
(fullscreen, resolution, AABB debug view, ...)

->I have no way to tell gamekit to load the created cfg file.

The solution I propose is this one:
-Set a correct usage form for ogrekit:
  usage: OgreKit FILE [CFG_FILE]

-If CFG_FILE is specified load it, if not check for a cfg file with the same 
name (except extension) in the same directory as the blend file.

Please tell me what you think and if it is OK for me to start working on this.

I attached the blender plugin in its current state for those who want to check 
it (not you will need to apply this patch 
https://projects.blender.org/tracker/index.php?func=detail&aid=22614&group_id=9&
atid=127 to blender)

Original issue reported on code.google.com by xavier.thomas.1980@gmail.com on 16 Jun 2010 at 8:25

Attachments:

GoogleCodeExporter commented 9 years ago
Aside from the exact specification on how you are planning to do it - I am for 
the idea.

As for the exact specification, I would suggest a flagged option, such as:
 OgreKit [-cfg CFG_FILE] FILE

This would allow for adding other options without there needing to be an order 
arbitrarilt applied. For example, at some point I want to be able to load 
"library blends" from a given directory, so would be adding a "-libdir DIR" 
flag to the options.

Original comment by BenT.Sol...@gmail.com on 16 Jun 2010 at 10:25

GoogleCodeExporter commented 9 years ago
Yes a cli switch would be better and I think the convesion is to use something 
like this:
-c  or
--config-file

Loading library blend is a must have but the libraries path is saved into the 
.blend (either relative or absolute) so such a flag as "--libdir" is not 
mandatory.

Original comment by xavier.thomas.1980@gmail.com on 16 Jun 2010 at 10:30

GoogleCodeExporter commented 9 years ago
On the implementation side, I will follow the wish of the project developers. 
But my ideas are: 
-If we just want to specify a cfg file we can just parse the command line 
manually to avoid introducing a new dependency.
-If we want clean command line parsing with possible further evolution there is 
a lot of of libraries with compatible lisence (Boost, CLAP, TCLAP, commoms CLI, 
...) From what I saw until now TCLAP http://tclap.sourceforge.net/ seems to be 
a good choice (lightweight, portable, MIT license, c++ ) unless the boost 
headers embedded in ogre code have the boost command lie parsing feature (which 
I doubt)

Original comment by xavier.thomas.1980@gmail.com on 17 Jun 2010 at 12:05

GoogleCodeExporter commented 9 years ago
FWIW - GameKit is not using Boost so anything relying on that would be out (I 
think). I've used STLPlus before for command line parsing (amongst other 
things) and it worked quite well. Not sure if we want to add it in only for 
that functionality though.

On the subject of library blends, I'm thinking that the path being saved might 
be an issue. But as this is a separate problem to this - I will raise an 
"issue" should it prove to be so :)

Original comment by BenT.Sol...@gmail.com on 17 Jun 2010 at 1:54

GoogleCodeExporter commented 9 years ago
Indeed, gamekit doesn't use BOOST or other bloated libraries.

Is there not already some command-line parser in the tree?

Otherwise, you could add tclap support. Just make sure it automatically works 
with CMake, just like the rest of gamekit.

Original comment by erwin.coumans on 21 Jun 2010 at 6:11

GoogleCodeExporter commented 9 years ago
I know gamekit does not use boost but I thought Ogre3D embed some boost header 
file (a really small fraction of them).

As far as I know gamekit does not use any library that handle command line 
parsing. Regarding parsing the configuration file (.cfg with startup parameters 
like resolution) gamekit is using Ogre::ConfigFile. I looked into ogre API for 
command line parsing utility and found nothing but I may have missed it.

Original comment by xavier.thomas.1980@gmail.com on 22 Jun 2010 at 6:48

GoogleCodeExporter commented 9 years ago
If Ogre embeds Boost in any way, it must have copy + pasted the code into their 
own headers (unlikely given Sinbad's leadership on the issue). I have not 
noticed anything Boost-ish in the basic Ogre code at all.

Original comment by BenT.Sol...@gmail.com on 22 Jun 2010 at 9:20

GoogleCodeExporter commented 9 years ago
When searching the code it seems Orge is related to boost in 2 ways:
-Permit to use boost thread (optional and other choices are available)
-OgreAny.h is a modified copy of boost::any.

Back to topic: does anybody knows if there is a command line parser in the 
source three that I missed? If not I will look into tclap. It should be quite 
easy to incorporate in cmake build system, in fact it does not even need 
compilation (c++ template with a unique header to include)

Original comment by xavier.thomas.1980@gmail.com on 22 Jun 2010 at 10:48

GoogleCodeExporter commented 9 years ago
OgreAny.h: So, in other words, Ogre "must have copy + pasted the code into 
their own headers", yes? 

Boost is still classified as too large a library to be a requirement for Ogre. 
Hence the threading is both optional (also unused in GameKit) and has a variety 
of alternative implementations such as POCO (as you mention). The most 
important part though is that it is "optional", hence I can understand why it 
is not used in OgreLite.

As for an Ogre built-in command line parser, I don't think there is any. 

Original comment by BenT.Sol...@gmail.com on 23 Jun 2010 at 12:57

GoogleCodeExporter commented 9 years ago
Here is a patch using tclap. It was really easy to set up in CMake and to use 
the API. More cl argument can be added later easily if needed. It is ready for 
review but I am ready to correct/modify whatever you feel is wrong.

Note the default argument (-v , -h and --) are automatically added by tclap but 
can be removed.

./OgreKit --help

USAGE: 

   ./OgreKit  [-c <string>] [--] [--version] [-h] <string>

Where: 

   -c <string>,  --config-file <string>
     Startup configuration file (.cfg) to use.

   --,  --ignore_rest
     Ignores the rest of the labeled arguments following this flag.

   --version
     Displays version information and exits.

   -h,  --help
     Displays usage information and exits.

   <string>
     (required)  Blender file to launch as game.

   Ogrekit

Original comment by xavier.thomas.1980@gmail.com on 23 Jun 2010 at 2:52

Attachments:

GoogleCodeExporter commented 9 years ago
Just had a chance to check out the patch. TCLAP has a lot of interesting 
features.

I wounder if we should add it to the gkUserDefs class and add the ability to 
specify most options from the cmd line ? 

bParse can also handle ID properties. Some GameKit options could also be saved 
in properties and parsed when loading the .blend

Original comment by snailr...@gmail.com on 24 Jun 2010 at 1:14

GoogleCodeExporter commented 9 years ago
I also agree that using more command line parameters (like fullscreen mode or 
the resolution) can be practical. Adding the command line parsing functionality 
to gkUserDefs is possible; but then it would not be very practical, for people 
developing their own program using gamekit, to add their own command line 
parameters.

As for the parameters that are also present in the blender file (fullscreen, 
resolution, debug mode, debug aabb, ...) I was thinking of using the blender 
"external renderer" addon for gamekit to export them to a custom 
/tmp/OgrekitStartup.cfg. Of course directly parsing those values from the 
.blend would be better but some like the resolution need to be parsed before 
the creation of the window. If not we need to re-dimension the window 
afterwards,but I am not sure this is supported on all plateforms.

Original comment by xavier.thomas.1980@gmail.com on 25 Jun 2010 at 12:58

GoogleCodeExporter commented 9 years ago
I just tested the TCLAP patch and it is pretty good. There is some minor 
weirdness in the error output (adds the empty option [--] which makes no sense 
to me) and I would make the blender-file parameter optional (to allow 
testing/debugging from the IDE). But they are minor things (one I ignored, the 
other I changed - so no big deal).

I'm thinking that the adding options to the Blender file itself would be a cool 
and useful addition. However, I think some things need to remain external, for 
example the screen resolution. Nigh on all games I've played have a fixed 
resolution only (hard-coded, not game-coded) for the device or allow the 
end-user to select the resolution from a list of detected/standard options.

When full-screening a game, I'd like to maintain the resolution options 
external to the blend file which would require checks and testing depending on 
the blend file loaded (multiple blends/levels, library blends, etc could make 
this a non-trivial task). Just an opinion though as, personally, I can just 
never put the resolution options in the file!

Original comment by BenT.Sol...@gmail.com on 25 Jun 2010 at 1:12

GoogleCodeExporter commented 9 years ago
Window size is a bit tricky, things like textures need the render system/ 
window to be initialized first. So it really is a hassle to specify those 
directly in the blend.

Exporting to an external config file is fine, but would eventually like the 
option for both. (Similar to AStyle: http://astyle.sourceforge.net/astyle.html)

The blend should always be the last option, and should not require a switch. 
This way drag & drop over the executable works. If no options are specified, 
the momo_ogre.blend should be loaded. (It's built into the OSX bundle, I 
believe...)

Original comment by snailr...@gmail.com on 25 Jun 2010 at 1:07

GoogleCodeExporter commented 9 years ago
Alternatively, we can use bParse twice: Once on the Main.cpp just to get 
resolution and fullscreen mode. Then close the file, initialise the window and 
reopen the file to load data.

Original comment by xavier.thomas.1980@gmail.com on 25 Jun 2010 at 3:04

GoogleCodeExporter commented 9 years ago
Loading the blend twice would be a performance hit. 

If the need is there to get window parameters from the blend, it would be best 
to restructure the application to load the blend first, grab the parameters, 
initialize Ogre, then convert the data all in one go. However, I agree that the 
window size parameters should stay external. Either in the .cfg or passed as a 
cmd line parameter. 

Initially, I'll get your patch applied in Main.cpp and go from there.

Original comment by snailr...@gmail.com on 25 Jun 2010 at 5:13

GoogleCodeExporter commented 9 years ago
I also think parsing the blend file twice or refactoring the application just 
for the resolution is a bit overkill. For me command line parameters and/or 
config file is ok.

I work slowly because I prefer to check with you here that I don't do stupid 
things but I have some free time to work on this project, so if you have some 
basic works/todo that I can help with (including continuing work on this patch) 
please do not hesitate to tell me. I can be contacted by private mail with my 
googlecode username @gmail.com and I also hang around on #blendercoders as xat.

Regarding the "--" argument, it can be removed easily.

Original comment by xavier.thomas.1980@gmail.com on 25 Jun 2010 at 5:50

GoogleCodeExporter commented 9 years ago
I disabled internal exception handling, so we can exit the app cleanly with 
--help,  and made the blender-file parameter optional. Also, made the 
gkUserDefs::parseString method public, so members can be set easily from the 
command line.

Any help is very welcome, so anything is up for discussion.

Some things todo could be, More sensors and actuators for the logic brick 
system; More bindings to Lua using swig;  Add / convert more constraints; help 
with isolating any bugs. We usually discuss GameKit activities, here or on 
Google Wave,  
I can send you an invite, if you don't have a wave account.

Original comment by snailr...@gmail.com on 25 Jun 2010 at 7:48

GoogleCodeExporter commented 9 years ago
On the issue of double-parsing the blend, I'm going to remain dead-set against 
that. If you only needed to parse the header or something, that could be hacked 
in but to parse an arbitrarily sized blend file for an option that is most 
likely not there (I mean, you only need it if you're overriding defaults 
right?) strikes me as very bad design. And this ignores the idea that I believe 
that screen sizes shouldn't be blend-file specific.

On the communications front - I also have Wave invites available should they be 
needed... That said, I was not aware there was a GameKit Wave.

Original comment by BenT.Sol...@gmail.com on 26 Jun 2010 at 10:33

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago

>> For me command line parameters and/or config file is ok.

We can have both config file and basic command-line handling.

I would focus on a config file, that is best supported cross-platform. Mac OSX 
and Windows users typically just double-click on the application (without 
passing command-line arguments).

Are there any issues integrating TCLAP?

Original comment by erwin.coumans on 28 Jun 2010 at 4:23

GoogleCodeExporter commented 9 years ago
No TCLAP have been integrated by snailrose and works great. For now the only 
argument is a path to a config file but it would be easy to add other command 
line argument that overwrite the default parameter or the one in the config 
file.

Original comment by xavier.thomas.1980@gmail.com on 29 Jun 2010 at 12:27

GoogleCodeExporter commented 9 years ago
OK, sounds good. 

Thanks for all the feedback and the patch!

Original comment by erwin.coumans on 29 Jun 2010 at 4:55

GoogleCodeExporter commented 9 years ago
Closing this, in svn for a long time.

Original comment by xavier.thomas.1980@gmail.com on 6 Aug 2010 at 6:00