xygine icon indicating copy to clipboard operation
xygine copied to clipboard

In game editor implementation

Open JonnyPtn opened this issue 7 years ago • 23 comments

Disclaimer: Most of this happened accidentally, at various levels of sobriety, without any real plan.

The biggest change is xy::App::getRenderTarget() replaces getRenderWindow(). This allows the editor to show the game viewport by switching out from a RenderWindow to a RenderTexture, without any change to the end users code. I also removed the RenderWindow* from State::Context, as it didn't seem necessary, and will enforce the use of getRenderTarget() instead

I also added an Editable base class, so other classes can inherit this then implement the editorDraw() method to show their own windows in the editor. I'm not entirely convinced this is the best way to allow classes to interface with the editor, it just happens to be the implementation I ended up doing.

I've added some quick example implementations in xy::Scene and xy::MessageBus and also updated the demo to work with the editor, So have a play around and let me know what you think. It's mostly a proof of concept at this point (you can't really edit anything...), so radical suggestions are certainly welcome.

This may be a longer term project, so if you want to set up a separate branch for it feel free, and I'll re-target this PR.

JonnyPtn avatar Mar 18 '18 22:03 JonnyPtn

CI is failing as sf::RenderTarget::setActive wasn't implmented in 2.4.2 seemingly... so you'll need to use latest SFML until I can solve that one

JonnyPtn avatar Mar 18 '18 23:03 JonnyPtn

Looking good! I agree, this probably should be in its own branch so I've created one called 'editor'. This is mostly because there are breaking changes and I'd like to get existing projects (Clamber) out the door first, so I don't need to worry about updating them. Might also be useful for waiting until the setActive() thing comes through and SFML release too so I can use package manager versions on macOS / linux (although that's no real biggie I guess)

fallahn avatar Mar 19 '18 10:03 fallahn

Yep, all sounds reasonable to me. SFML 2.5.0 should be out imminently so can retarget at that point.

I've got a few more things which should be straightforward copies from the standalone editor, which I plan on doing later today.

JonnyPtn avatar Mar 19 '18 16:03 JonnyPtn

Have you got any thoughts on how one would save a scene to a file? I've been attempting to implement something, but struggling to wrap my head around how the scene would "collect" all it's entities together.

JonnyPtn avatar Mar 25 '18 19:03 JonnyPtn

Hmm. Initial thoughts are:

Data driven approach

The config file format could, potentially, work as a storage format, although it would require a few things. The advantage would be that any xygine application could read it at any point, and the editor specific system can be used to write it (as, in theory, it has access to all editable entities). As components are effectively just data then they can be stored as objects/properties in the config file with little problem BUT creating IDs is where I get stuck. For stock components this isn't too bad I guess, but custom components would need some way of generating and decoding an arbitrary ID during serialisation, so that the correct data type can be created when loading. Using typeid might be a start, but I'm not sure how much is implementation defined, and whether this would cause IDs to be generated differently between different compilers. Another idea I had was based on the Id/Valve approach of .fgd files - configuration files which identify particular entities as being available for the worldcraft/hammer editor. This approach would mean creating a configuration file loadable by the editor that lists available components/systems for the currently active application.

Finding the components on a particular entity can be done in the entity manager, by querying the componentMasks array at the entity index, which returns a bitmask of flags, updated when a component is added to an entity (see here).

Some sort of reverse lookup would be required to identify systems needed in a scene when adding a specific component type for the first time. Systems have a bitfield, marked with System::requireComponent<T>() so, again, all systems including custom types need to be listed somewhere such as an .fgd style configuration, and enumerable so that the editor can check the list for any new systems required by a freshly added entity/component. Systems would probably also need some sort of priority ID which hints at which order they should be added to a scene - for instance text renderers need to be added after the graphics renderer. This could potentially be a problem when a user has added a text component before anything else - I suppose the solution to this is to add support to the Scene class to sort the list of drawable system pointers according to their given priority. This way particle systems and text renderers can be ensured to appear over the top of the other rendering systems.

Binary file approach

There is also the possibility of serialising a scene as some sort of binary format using a library such as cereal although I haven't looked too deeply into it. This way would potentially store the state of a Scene as it exists in memory, and restore it again on load. I don't know what advantage/disadvantage this would have over basically recreating the scene in a data driven approach as above, although I feel it might potentially be less error prone because it prevents users from easily modifying the save files with arbitrary data (ie typing bullshit into a config file) making it more robust when it comes to error handling on load.

Hybrid Approach

The list of entities in the Scene, and their respective components could be stored in the ConfigFile format - but the components themselves can be serialised into binary files, references to which are stored in the configuration file. This would probably help with more complex components (but not sure how it would work with components such as Transform which contain pointers to other transforms in a SceneGraph scenario - I need to check the cereal lib docs first). This may or may not help with making sure to preserve the data types of components - I've kind of tagged this on here as an afterthought :D

fallahn avatar Mar 26 '18 09:03 fallahn

With the latest commit, The sprite editing is in a state where it could be considered ever so marginally better than manually editing the sprite files...

The code is still a bit of a mess (although I've tried to keep the messy code self contained within the Editor hpp/cpp files)

I've loosely added (and not tested) an XY_EDITOR macro to enable/disable editor stuff. When configuring with cmake make sure to add "-DENABLE_EDITOR=TRUE"

JonnyPtn avatar Mar 29 '18 21:03 JonnyPtn

How mad would it be to suggest making the editor system not require any components? This gives me a fairly nice way of accessing every entity in the scene

JonnyPtn avatar Apr 10 '18 23:04 JonnyPtn

Would that work? Entities ought to only be added to a system should the component mask match the requirements. If no requirements exist then I'd assume no entities would be added to the system (and if they are then this is probably a bug ;) )

fallahn avatar Apr 11 '18 08:04 fallahn

It appears to work :S I may have already implemented it in the latest commit for this PR...

If you open the editor in the demo and go to the Scene editor, it shows all the entities in the scene.

Given that It solves a problem for me, I'm going to claim this bug as a feature \o/ . I don't think it would be unreasonable to assume that if a system doesn't require any specific component then it will operate on all entities, and I can't think of a scenario when this behaviour would cause a problem?

JonnyPtn avatar Apr 11 '18 16:04 JonnyPtn

No, I guess it's not a problem - the requirements are really only there so that elsewhere in the system it's safe to assume an entity has a specific component without having to check with hasComponent() first

fallahn avatar Apr 11 '18 16:04 fallahn

RE Scene saving/loading, using cereal seems good (I'm very impressed by it's crazy template black magic...) and it has the option to choose between binary and human-readable formats, so I've got a rough proof of concept working using that.

There's a few problems I've been thinking over for a while:

  • For components which require resources (for example Sprites which require a texture, and sounds which require an audio file) how should I serialise that data? My first brute force method was to just add the file path as a member var on the component, which I know adds quite a bit of weight to the components, but I can't get any fancier methods working
  • entity ID consistency. You touched on it above, and I've fiddled with a few methods, none of which are completely convincing. E.g. only allow scene loading on object construction which forces the loaded entities to always have the same ID's, or adding a createEntity(int ID) overload which create's an entity with a specific ID...

JonnyPtn avatar Apr 22 '18 00:04 JonnyPtn

Hm, the resources do pose a problem. I'll give this some further thought (and perhaps ask about a bit). As for entities, assuming the Scene class is loading the file then it also has access to the entity manager. It shouldn't be a problem to add the createEntity(ID) overload to that, rather than adding it to the Scene class itself, keeping it away from the public API

fallahn avatar Apr 23 '18 09:04 fallahn

RE resources - perhaps it might be possible to have a table which contains resource paths and an ID which marks the path as the kind of resource it is, ie font/shader/texure. Then the index of the table entry can be stored with the component, without adding too much bloat to it? The only downside really, apart from the sheer amount of work needed to implement it, is that it forces a requirement on a custom component that it should provide one or more resource IDs, which is a shame because one of my favourite things about the current system is the fact that you can use almost any arbitrary data as a component.

fallahn avatar Apr 23 '18 17:04 fallahn

The other option, which is loosely implemented in latest commit (Only really committed so I could pull the other changes) would be to serialise the texture data in the scene file to create a sort of "asset pack". cereal apparently works with smart pointers (not raw), so changing the resource handlers to use shared_ptr makes this work (but of course that would be another fairly major change to the current xygine API). This would also allow the possibility for the end user to create components with (smart) pointers to whatever resources they want, even custom ones, as long as they provide an overload of the cereal template serialise functions (e.g. here where I knocked together some quick-n-dirty functions for sf::Texture)

JonnyPtn avatar Apr 24 '18 15:04 JonnyPtn

Either way this means the resource management part of xygine needs reworking. For a while now I've also had vague notions of updating the interface to work with resource IDs rather than strings containing paths (outside of initial resource loading, at least). If this isn't a roadblock for you I want to spend some time considering a design for an overhaul, before running it past you to see what you think.

fallahn avatar Apr 24 '18 15:04 fallahn

Sure, no roadblock as there's plenty of tidying and refining to do on other parts of this PR

JonnyPtn avatar Apr 24 '18 16:04 JonnyPtn

OK, this is my pass at the problem. I figured what really needs to be stored in the editor data is the source of resources, ie paths to files on disk etc, rather than the resources themselves. This means that:

  • This is compatible with the current resource managers and components, as well as custom components
  • Components only need to store an extra integer ID
  • Users won't have to provide any custom serialisation - it's all performed on the directory class
  • No shared_ptr
  • Assets remain unencoded on disk and can be updated without reserialising
  • Entities/components aren't required to use this outside the editor so coding directly remains flexible

The only downside currently is that it doesn't support shaders because they're a bit more complicated than loading straight from disk (just look at the shader resource implementation) - but then I don't know how you plan on handling them in the editor anyway :)

fallahn avatar Apr 25 '18 11:04 fallahn

So I had a play around with the resource stuff, and came across a few issues when trying your potential implementation, so for now I've just implemented a simpler method which uses an ID created by hashing the resource path, which works for what I need to save/load scenes and fulfils your bullet points mentioned above.

I've also fixed up the CI so it builds correctly on *nix systems, although the windows build seems to be getting confused with my ResourceID (using ResourceID = std::size_t; seems to be implicitly converted to sf::Uint32 or something?)

I'm going to start work on filling in the missing implementations and tidying up a little bit now. The UI/UX of the editor leaves a lot to be desired, but I may leave it until the functionality is more complete, then worry about making it easier/smoother to use.

If you want to start reviewing the code and pick through any bits that look odd (there's probably quite a few), feel free to question anything, then it's up to you at which point you want to merge into your editor branch, which we can then take a more granular/measured approach to improving/fixing it up.

JonnyPtn avatar May 18 '18 20:05 JonnyPtn

Looking good! I've added a few commits just to update the visual studio project and get everything building on Windows. The only query I have is that here you appear to be returning a pointer to a local variable :) I'm going to try out the linux build presently, after I've fixed arch which has decided to implode itself...

fallahn avatar May 19 '18 11:05 fallahn

Yes, that is one of many curious code decisions you'll come across, which I'm working through currently. That particular one I had seen the warning for and promptly ignored. If memory serves me it was a temporary solution, and what that function should actually be returning is a std::optional

JonnyPtn avatar May 24 '18 17:05 JonnyPtn

RE: the demo no longer launching on windows - replacing the renderBuffer and textureResource globals in Editor.cpp with unique_ptrs and creating/destroying then in Editor::init() and Editor::shutdown() worked around the problem. I can commit/push the changes or let you handle it if you prefer :)

fallahn avatar Jun 01 '18 09:06 fallahn

Hi, are you still woking on this?

zerodarkzone avatar Mar 07 '19 01:03 zerodarkzone

I haven’t been actively working on it for a while, but I do still intend to pick it up again soon(tm).

Have considered moving back to a separate project again, just to make it easier to manage

JonnyPtn avatar Mar 07 '19 09:03 JonnyPtn

intend to pick it up again soon

Road to hell and all that... Going to close this for now as I want to clear up some of my todo lists and I don't think I'll be picking it up again any time soon

JonnyPtn avatar Oct 17 '22 09:10 JonnyPtn

Hey, thanks for all your help on this and all your other contributions to xy over the years - it's much appreciated! However, while xy is still a very capable engine I've moved all my development efforts over to crogine now - so I appreciate any further efforts on this may be wasted. Not a complete loss however, I did cherry pick a few chunks of code over the years and use it elsewhere in both xy and cro!! Thanks again 😁

fallahn avatar Oct 17 '22 10:10 fallahn