ZDoom

[quote="AFADoomer"]Is the test file compatible with the dev build implementation? I'm using the 26 Jul build - ge2778ba44.

The SwapModelDef and SwapSkins actors both end up invisible for me, and I've had no luck just swapping definitions or skin textures in my own tests (actor always ends up invisible unless I fully define the model path as well).

Model swaps seem to work OK, though.[/quote]

Should be fixed as of this PR

https://github.com/ZDoom/gzdoom/pull/1680

Is the test file compatible with the dev build implementation? I'm using the 26 Jul build - ge2778ba44.

The SwapModelDef and SwapSkins actors both end up invisible for me, and I've had no luck just swapping definitions or skin textures in my own tests (actor always ends up invisible unless I fully define the model path as well).

Model swaps seem to work OK, though.

As of this comment, it would appear that all checks have passed. Additionally, I believe have once and for all snuffed out any undefined behavior.

It still emits this when compiling on Mac:

/Users/runner/work/gzdoom/gzdoom/src/playsim/p_actionfunctions.cpp:5085:27: error: cannot pass non-trivial object of type 'FString' to variadic method; expected type from format string was 'char *' [-Wnon-pod-varargs]
fullName.Format("%s%s", modelpath, model.GetChars());

Must be 'modelPath.GetChars()'

Okay, I have submitted a commit handling the serialization. I've also managed to make a TArray to store Model paths/file names of models that were loaded by A_ChangeModel so that when the save file is loaded, it knows to push those back into the model array where they belong.

I have submitted another commit to address the requested changes, and posting here for extra visibility.

[url=https://github.com/coelckers/gzdoom/pull/1651#pullrequestreview-1022886062]You have more to go.[/url]

If you're curious, it [i]sounds[/i] like Graf might be asking you to do what I had done with ViewPos. I could be completely wrong here. Specifically, I created a ViewPosition class that is only initialized when [b]SetViewPos[/b] is created.

You can see how I did that [url=https://github.com/coelckers/gzdoom/pull/1517]here[/url], with [url=https://github.com/coelckers/gzdoom/commit/27cba4c99038c8bbc378b9ce1000ac389130803b]some additional fixes by Graf[/url] that must be considered.

In your case though, it should be much simpler since you're not exposing anything else to ZScript so you should be able to create a class or struct for these, then just make sure they're serialized, no need to worry about IMPLEMENT_CLASS and DEFINE_FIELD_X.

I have updated the test file and submitted a commit which I hope is sufficient for dealing with the overhead.

It would be better if this was offloaded to an external data structure. We really need to get stuff out of AActor, not more in.

I can argue that most people aren't going to use 16 indices for a model.

I've done a bit of research, and it looks like I can just use 4 indices instead (because that should be more than enough in most cases, and what the max used to be). If I'm doing my math right, then that would use 36 bytes instead. Would that be better?

Sorry, but no, not like this. You are adding 132 bytes to AActor just for a feature most people won't ever use. That's plain and simply unacceptable.

[url=https://github.com/coelckers/gzdoom/pull/1651]Pull Request[/url]

A_ChangeModel(string modeldef, int modelindex string modelpath, string model, int skinindex, string skinpath, string skin, int flags, int generatorIndex)

This can change the modeldef, model and skins of an actor.

You can add models to an actor to render by using generatorIndex to tell your new model index to copy the behavior of another one. You may only copy the behavior of a model index already defined in your actor's current modeldef. To remove a model or skin, just pass "" in model/skin parameter to revert to the default.

Available flags:
CMDL_WEAPONTOPLAYER - If used on a weapon, this instead change's the model on the player instead.
CMDL_HIDEMODEL - Hides the specified model index from rendering. Can be used with CMDL_WEAPONTOPLAYER.
CMDL_USESURFACESKIN - skinindex instead corresponds to the number of a surface to replace it's skin.

https://drive.google.com/file/d/1iz6_BT0JvGnao3l8T8t40mcEMjHa9tHN/view?usp=sharing

Here is a test file. Let me explain the various things you can do with it:
[b]Summon SwapModelDef[/b] - This shows off dynamically swapping modeldefs
[b]Summon SwapModels[/b] - This shows off raw dynamic model swapping
[b]Summon SwapSkins[/b] - This shows off raw dynamic skin swapping
[b]Summon SwapModelsAndSkins[/b] - Dynamically swaps models and skins
[b]Summon SwapModelsEdgeCase[/b] - No modeldef is defined, will not show any models even though a model and skin is given
[b]Summon SwapModelsEdgeCase2[/b] - It tries to switch to hammer, but has no modeldef. Therefore, it just shows up as a pistol sprite
[b]Summon SwapModelsEdgeCase3[/b] - It tries to switch to hammer, but has no modeldef. Therefore, it just shows up as a pistol sprite. But 1 second later, it will also use CMDL_HideModel. This draws a model at index 0, but it won't render.
[b]Give TriggerSwapGun[/b] - This token changes the weapon view model and player third person gun model to show off the ideal usage of CMDL_WEAPONTOPLAYER
[b]Give TriggerSwapPlayer[/b] - This token changes the player model.
[b]Give TriggerUnSwapPlayer[/b] - This token changes the player model back to the default. Basically just a different way to revert to default without constantly forcing it.
[b]Take TriggerSwapModel[/b] - This token reverts the player model and weapon view model back to default. This is where the bug I mentioned occurs, but can be circumvented by just manually stating the model to revert back to.
[b]Switch to pistol[/b] - This makes the player's gun model no longer render.