Skip to content
This repository has been archived by the owner on Jan 25, 2023. It is now read-only.

Extended Prefab Support #2213

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Extended Prefab Support #2213

wants to merge 13 commits into from

Conversation

SirNate0
Copy link
Contributor

I added additional prefab support in loading nodes. Instead of just loading prefabs with Load__() in code, you can now add a prefab attribute specifying a file in the data, and then overwrite attributes or add additional components/child nodes as need. Works with LoadXML (and in theory LoadJSON, though I haven't tested it). See the changes in the scene loaded by Sample 38 for an example.

Naturally, the loading changes were the easier part. Altering saving to work with this is much more difficult, as to have it be useful you have to load the prefab file again and then only save the differences (e.g. position will almost certainly be different). I do intend to try and solve this (probably save the data to XMLElement/JSON value and then write some sort of Diff() function to compare the results and save only the changes, or to just save it directly, not as a prefab, if there were, for example, removals. I may also try extending it to allow specifying id mappings or the removal of nodes/components from the prefab.

However, I primarily use the Blender Exporter, so these changes are already as useful as they will likely be for me, as I don't use Urho's scene/object saving functionality, only loading. In addition, I think, for at least xml, the same can already be accomplished with xpath stuff, so I'm not certain how worth it such changes would be.

Also, presently this depends on my relative path stuff (pull #2070), but it doesn't need to. In fact, these changes with just loading don't use the changes beyond the possibility of using relative paths in specifying the prefab file. (It would be easy to change it to work without the relative path changes -- just drop the source.GetBasePath() parameter from GetResource in the last couple of commits). However, with saving being added the addition of node storing it's load path will likely be needed, so the changes to support relative paths will likely be more important.

Any thoughts? And if you want, this can be merged as is once the relative paths stuff is finished, and then I can just create another pull request if I get around to the saving stuff. If you want someone can also cherry pick the changes for just the prefab loading and make the correction I mentioned above with a new pull request.

SirNate0 added 13 commits July 10, 2017 21:27
UIElements and Nodes store the paths of the files they were loaded from.
LoadJSON presently cannot do that, as it does not have access to its path.
Handling with scripting still has some things that need to be decided.
Should Node.CreateScriptObject be relative to the Node's path or to the Script's?
Likewise, how to handle ScriptInstance's loading of a ResourceRef attribute?
Still remaining: LoadJSON base path handling, deciding what to do with scripts for the base path
@organicpencil2
Copy link

Nice. I feel like everyone (except for that one guy who is reading this and saying "not me") has needed to hack-in similar functionality at one point. It'd certainly be convenient to have in core, though I've yet to see a one-size-fits-all solution.

Concerns:
-Adds xx bytes per node (whatever an empty string is) regardless of whether a person intends to use prefabs.
-Duplicated string paths will add up as more prefabs are instanced. Could save memory by caching the paths and storing a hash, but that still doesn't truly solve the first problem. Or maybe I just have programmer OCD and the first problem isn't truly a problem...

I don't really have a good answer, except perhaps writing the implementation in script using node vars. But again - it would be very convenient to have something like this out of the box.

@SirNate0
Copy link
Contributor Author

That's actually a good point -- the path could be stored in the node's user vars -- something like prefab_path or such. Then there aren't the additional bytes required and since it is just during saving that the variables are required to be used the cost in looking up the variable in the map shouldn't be an issue.

@github-actions
Copy link

Marking this stale since there has been no activity for 30 days.
It will be closed if there is no activity for another 15 days.

@github-actions github-actions bot added the stale label Mar 21, 2020
@weitjong weitjong added backlog and removed stale labels Mar 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants