Animcomposer and external change refactor

[neph1]

I'd like some opinions on this. Primary goal is to replace AnimComposer when updating the gltf, which it seems to do, although you need to reopen the .j3o to see it.

But mostly I'd like to know your thoughts on the refactoring. I'm thinking it can be taken further and maybe have a selectable list of transfers the user might want to do, rather than automating everything. It's out of scope for now though.

[neph1]

Since I don't know how to make a checklist option, I added an option to either apply all data or only the mesh data (or none). Should satisfy most needs. I don't like that it loses the option to select whether to only apply mesh and animation data (like before)

[neph1]

Not sure what happened here, I must have missed adding a couple of files before committing, then made a hard reset of the branch because they were missing locally too. The dangers of context switching I guess.
I think this is done feature wise. Not sure I'll be able to fix all those codacy issues, though.

[neph1]

Pause this for a bit. I found an issue while testing.

[neph1]

I hadn't checked before that animations actually worked, just that they had been updated. I've fixed that now, however, they don't work in the scenecomposer (without reloading the j3o). My guess is that the JmeAnimComposer needs to be updated with the cloned AnimComposer. Since you're the author, @MeFisto94, is there a convenient method of doing such things?

Edit: I've sort of semi-verified that it's something like that, by not deleting the old AnimComposer and just cloning the fields unto it. That could be a work-around, but I'm unsure if it could cause other issues, since the old code relies on deleting all controls for the node tree.

[MeFisto94]

I don't remember too much of it, just that the JmeXyz are counterpart to engine Xyz classes and basically are mostly there for having Java Bean Properties and a tree structure. We had this refreshChildren thing on JmeNodes or something, but I don't know if it fixes that. Technically, the cloned AnimComposer is a new AnimComposer that should have a new JmeAnimComposer that is then put into this tree hierarchy and refreshed, I think.

So instead of trying to change stuff in there, I guess the official way would be to kinda reload the node tree in some way (but I don't remember API specifics there)

[neph1]

I did look for a way to refresh the node tree. I assumed there would be a similar problem with other things, but didn't find it. I'll give it another try, though.

…

On Sun, 1 May 2022, 20:23 MeFisto94, ***@***.***> wrote: I don't remember *too much* of it, just that the JmeXyz are counterpart to engine Xyz classes and basically are mostly there for having Java Bean Properties and a tree structure. We had this refreshChildren thing on JmeNodes or something, but I don't know if it fixes that. Technically, the cloned AnimComposer is a new AnimComposer that should have a new JmeAnimComposer that is then put into this tree hierarchy and refreshed, I think. So instead of trying to change stuff in there, I guess the official way would be to kinda reload the node tree in some way (but I don't remember API specifics there) — Reply to this email directly, view it on GitHub <#316 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB46MQWMAVTU5HPOQ4Q43CTVH24YRANCNFSM5RUTRLKA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[MeFisto94]

Looking at #83 this should be possible.
Something like AbstractSceneExplorerNode#refresh. And ideally those node classes extend that.
If that doesn't help, give me a ping and I do a more thorough search

[neph1]

I think this is finally done. It's not an elegant solution, but at lest it seems to work. If someone wants to test it:

1. Convert an asset of your liking (preferably with animations
2. Open it in the SceneComposer
3. Edit it in Blender and save (preferably something to do with animations, just change default anim)
4. Wait for ExternalChangeScanner to kick in
5. Choose what to update (Animations is the most critical part)
6. If you changed default anim, you should notice it updating in the 3D view. You should also be able to change animations in the SceneExplorer.
7. Feel free to test the other options too.
   Something you can't do is delete nodes and update, it has nothing to go on then. But it was like that before this PR, too.

[MeFisto94]

I'd love if someone would help in testing this PR :) I'll still need to review it, but since it's rather large, I probably need to schedule some time for it

[neph1]

It may not be as big as it looks. Most of the code for Mesh and Animation transfer is taken from SpatialUtil, only broken out into separate classes, and Material and Transform transfer is built on the same "pipeline".
I've tested everything quite well, but since the Animation transfer is newest, and I was a bit exhausted with the whole thing, there may be edge-cases I didn't cover.
Maybe I should do another documentation commit.

[neph1]

@MeFisto94 If you want to, we could try scheduling an hour and do the review together. Regarding testing, perhaps we can push out a beta to let others try it.

[MeFisto94]

Looking good, there are just a few things that need fixing and those mostly apply to all classes from the datatransfer packet: Wrong LogLevel Imports and maybe considering giving them a different name. They may not come from your PR directly, but I guess we can fix them easily anyway

[MeFisto94]

Should we create an issue for that and maybe mention that in the TODO comment and vice versa?

[neph1]

Yes, that is an old comment, but I think it's still valid. But it's probably a rare use case.