Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MyGUI plugin for OpenMW resources #261

Merged
merged 11 commits into from Sep 14, 2014
Merged

MyGUI plugin for OpenMW resources #261

merged 11 commits into from Sep 14, 2014

Conversation

ghost
Copy link

@ghost ghost commented Sep 11, 2014

Previous attempts to use MyGUI's tools (LayoutEditor, SkinEditor, FontEditor) were not successful because it can not use resources from BSA files. Even after manually extracting them, it still wasn't working, due to backslashes use in path references.

I added a MyGUI plugin that can be loaded by the MyGUI tools and will register Ogre resource groups/archives for BSA archives and the MW data directory, according to the user openmw.cfg, just like it is done in OpenMW. It also registers some custom widget classes used by OpenMW so that they show up as expected.

As always, was only tested on Linux. The install rules need testing on all platforms. I did not add any install rule for Mac, because I could not find any for the other targets either. @Corristo?

@ghost
Copy link
Author

ghost commented Sep 12, 2014

I need a suggestion where to put the plugin sources. It can't go in libs/, because libs are not allowed to depend on components. But it can't go in apps/ either. A shared library is not an app. We might need a new top level directory.

@psi29a
Copy link
Member

psi29a commented Sep 12, 2014

we have libs which are static libs, so maybe we should have a new top level for shared libs, like /slibs or if this is a one-off/unique case, then another place /extern like where shiny is?

@kcat
Copy link
Contributor

kcat commented Sep 12, 2014

Perhaps "plugins/". Could also move openengine and platform to a different sub-directory, leaving "libs/" for libraries built from the components (like apps is for apps).

@nikolaykasyanov
Copy link
Member

@psi29a
Copy link
Member

psi29a commented Sep 12, 2014

I thought about "plugins" as well, but that makes it seem optional or interchangeable in some way.

@zinnschlag
Copy link
Contributor

+1 for plugins. Its a good description.

@@ -190,7 +192,7 @@ namespace MWGui

mSleeping = canRest;

dynamic_cast<Widgets::Box*>(mMainWidget)->notifyChildrenSizeChanged();
dynamic_cast<Gui::Box*>(mMainWidget)->notifyChildrenSizeChanged();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why to use dynamic_cast if its result is always supposed successful? It is necessary either use static_cast or check its result for nullptr

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possibility is if you want to easily catch a bad cast. If the dynamic_cast fails, it will give nullptr which will cause the call attempt to crash with a null this pointer, whereas a static_cast will give it a bad-but-maybe-usable this pointer.

We could probably make a template function that does a dynamic_cast, but throws a runtime exception if the result is null.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably make a template function that does a dynamic_cast, but throws a runtime exception if the result is null.

MyGUI already has such a function (Widget::castType). I will change all dynamic_casts on widgets to use it instead.

@ghost
Copy link
Author

ghost commented Sep 12, 2014

@scrawl https://github.com/OpenMW/openmw/blob/openmw-0.31.0/CMakeLists.txt#L682 here it is

Yes, but where are the different binaries installed? Is that done automatically in some way and I don't need to add anything?

@nikolaykasyanov
Copy link
Member

@scrawl how is this plugin supposed to be used? I mean, what should user do after downloading OpenMW with this stuff?

@ghost
Copy link
Author

ghost commented Sep 12, 2014

The plugin needs to be manually copied to MyGUI's plugin folder. It doesn't really matter where we install it, it just needs to be accessible.

@psi29a
Copy link
Member

psi29a commented Sep 12, 2014

wait.. what? OpenMW is going to be shipping it's own shared lib now (for MyGUI that is specific to just OpenMW)? I'm wondering about the Debian/Ubuntu packaging process here, I'm sure it is doable I just want to make sure this what you are telling us. :)

@ghost
Copy link
Author

ghost commented Sep 12, 2014

Yes, all distributions of OpenMW should contain this shared library. That is what I am proposing.

@ghost
Copy link
Author

ghost commented Sep 13, 2014

I'm wondering if it would be a good idea to have the code for the various custom widgets and the font loader within the plugin itself, and not in components/ as they are now. This means OpenMW would need to either load the plugin, or link against it at build time.

Pros:

  • Forces all packagers to include the plugin library, otherwise OpenMW won't build/run
  • Reduces overall size of binaries (shared vs static). Currently widgets and fontloader are included in the "components" static library and statically linked into both OpenMW and the plugin.

Cons:

  • If dynamic loading is used, code needs to be written to properly locate and load the library at runtime
  • If linking against the plugin is used, this could be considered unclean (linking against a library with no "lib" prefix, that was supposed to be loaded at runtime)

IMO linking against the plugin would be a good idea, but I'm open to other suggestions.

@zinnschlag
Copy link
Contributor

Not much on the pro side IMO. Binary size hardly matters. And forcing packages to include the plugin seems unnecessary.

@ghost
Copy link
Author

ghost commented Sep 13, 2014

And forcing packages to include the plugin seems unnecessary.

Why not? It's surely better than going after each individual packager and yelling at them to include it. This plugin must be obtainable for each OpenMW user, or they will miss out on an important feature.

@zinnschlag
Copy link
Contributor

We don't use any of this at this point, right? This would be for future UI mods, which is a post-1.0 feature for which we don't have a concept yet. As least I don't. Forcing stuff on people that we don't even know yet how/if it will be used seems wrong.

@ghost
Copy link
Author

ghost commented Sep 13, 2014

UI mods are not a post 1.0 feature. They have been possible since... ever? Just pick one of the GUI layouts or skin files and change them. The MyGUI tools greatly facilitate this process.

Of course there is more to think about, content file integration, refactoring/generalizing widget use in OpenMW code, which should be done post 1.0.

@zinnschlag
Copy link
Contributor

You can modify the GUI by changing the MyGUI resources (basically hacking OpenMW). Not the same as modding the contents of the data directories (via OpenCS and external tools that are used to create art assets).
There may also be implications regarding the post-1.0 resources handling improvement (moving to a different archive handling approach). I feel we are getting ahead of ourself.

Anyway, I don't really like the idea of making OpenMW depending on a plugin. That is a break from our current architecture. If there is code that can be meaningfully shared by both OpenMW and a plugin used by MyGUI tools it should go into components. The plugin is okay, but it should serve as an interface to the MyGUI tools only.
And I think this plugin should go into its own package. Shipping it with OpenMW would be like shipping a NIF exporter for blender with OpenMW. I suppose that would make it also easier for the packagers.

@ghost
Copy link
Author

ghost commented Sep 13, 2014

I still disagree, but not strongly enough to insist on it. Go ahead and merge it as-is, then.

Regarding the term "hacking" - any mod is basically a hack. Of course a mod may need to be updated whenever the parent content has been updated. In this case, GUI mods created in the way I described would not necessarily be compatible with all versions of OpenMW, but there's really no way around that.

@zinnschlag
Copy link
Contributor

Regarding the term "hacking" - any mod is basically a hack. Of course a mod may need to be updated whenever the parent content has been updated. In this case, GUI mods created in the way I described would not necessarily be compatible with all versions of OpenMW, but there's really no way around that.

That's what I meant. Modded content should be compatible with all (not outdated) versions of OpenMW. That's why I called it hacking.
Of course there can also be incompatibilities between content files (and the associated art assets). These kinds of issues are acceptable. OpenMW compatibility issues are not.

Anyway, merging ...

@zinnschlag zinnschlag merged commit bd96764 into OpenMW:master Sep 14, 2014
@ghost ghost deleted the myguiplugin branch October 8, 2014 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants