Use a separate player.tscn for each playable character?

[wjt]

We currently have a single player scene, at res://scenes/game_elements/characters/player/player.tscn. By default it has a generic/template appearance, placeholder name, etc. with various properties to customise it. In the main game, we override the Player Name in every scene to StoryWeaver (in theory!) and set the Sprite Frames property to make it look like StoryWeaver; in Stella and the Missing Star, in each level we set its name to Stella and adjust its SpriteFrames; and so on.

(We use the same approach for enemies, hint signs/bonfires in the sequence puzzle, etc.; everything I am writing applies to those too but I'll talk only about the player by way of a concrete example.)

I think this approach is flawed. As soon as you need to customise something that is not exposed as a property on player.gd, then the "correct" thing to do in this model is to use "editable children"; but it's much easier to click through to the player.tscn and edit it directly. We saw many people doing this while building a StoryQuest with a custom character, adjusting the hitboxes etc. for their character and inadvertently breaking StoryWeaver in the process. Even if you do use editable childen, if you try to edit the collision shape, you are still actually editing the resource in the canonical player.tscn. You have to remember to Make Unique on the shape before editing it. This is very easy to get wrong.

It's also error-prone if you want to change a property of the playable character across many scenes. If we decide to rename “StoryWeaver” to “Storyweaver”, or change their speed, then we would have to edit the Player node in 9 scenes; this number will only go up.

And if we want to add logic that is specific to StoryWeaver – such as outfit customisation – we would have to do this in a way that is generic across the player-characters in every StoryQuest.

I think we should take a different approach:

• StoryWeaver should be defined in a storyweaver.tscn scene
• Stella should be defined in a stella.tscn scene
• We should also have a template_player.tscn scene

These scenes would reuse the same set of scripts, but otherwise be completely separate.

Then, to make a custom player for your StoryQuest, you would duplicate template_player.tcsn (which would duplicate all the collision shapes, etc. contained within the file!), and then modify that. We could even go so far as to define the template's SpriteFrames as a resource embedded in that scene, rather than saved to a separate .tres file, so then the template would be self-contained except for the assets it references.

(StoryWeaver would keep their separate-file spriteframes.)

We would then remove the top-level sprite_frames property that proxies to the internal PlayerSprite, because you would set the frames there directly.

I think this would be less error-prone and more flexible. Thoughts?

[wjt]

The obvious downside is that any change to the player scripts that requires corresponding changes to the scenes will have to be made across all scenes in lockstep. For instance, using the new InputWalkBehavior node.

[wjt]

I filed #1151 for switching the player over to InputWalkBehavior - better done sooner rather than later.

[manuq]

Yes, I totally agree. This is separate but related to having behavior components for more flexibility. For the minimum viable product of the game we tried our best to make the important scenes "moddable" (like player, guard, etc) by exporting as many properties as possible and even proxied inner properties of the scene's nodes (like the SpriteFrames of an AnimatedSprite2D inside the scene). I think we did it that way by following the requirement of adding "templates", and probably based in our experience in the Moddable Platformer and Moddable Pong projects. But StoryQuest creators have shown how limited is it to change the visuals but not the hitbox of a character.

[jgbourque]

If they were to get their own separate copy of a character .tscn, would the script it uses be a copy of or the exact same script shared between it and the lore based storyweaver.tscn? I am just thinking if they want to adjust resources like the collision shape, for example, they'd likely also like to mod aspects of the script. This would give them all the functionality of the character scene without anything tying them back to it, right?

If that's the case, will the script they get for their separate player scene still contain all the variables exported to the inspector that allows them to lightly adjust scene mechanics and behavior as they can in the current templates? I would imagine that would all be the same as it is simply part of the script, yes?

If so I think this could be a good improvement towards really giving them their own character or component through and through - as opposed to a character still sharing resources underneath the hood.

[wjt]

No, I was thinking that the script files would continue to be shared.

[wjt]

IMO anyone proficient enough to modify GDScript can also figure out how to make a copy if needed.

In addition, the idea with the character behavior scripts we're working on is that there will be more, smaller, scripts, so less duplication would be needed to make a change to one part of the logic.

[jgbourque]

Ok this makes sense to me. Detaching and re-attaching a fresh script might not be a super heavy lift so we can see how that works first.

[wjt]

I decided to try doing this, but start small and move the sequence_puzzle_object.tscn into the NO_EDIT template. Rationale:

1. This scene is small
2. Over the last few weeks I have been fixing issues with this component & found it easiest to, e.g., make dedicated scenes for the tortoise in stella, the rocks to hop across in champy, and the musical rock itself.
3. In particular it was annoying that, when I wanted to reset the scale on the rock, I had to go through every quest and make sure it wasn't broken by this change. If each quest had had its own copy of the object scene, this would not have been necessary.

However I realised that I would have to extend the copying tool to handle this new case where, after duplicating scene A as A', in scene B we have to replace instances of A with A' before saving that as B'.

This is trickier than the other cases we handle so far because we need to handle the replaced node having properties set on it, child nodes (e.g. the Player scene instances have Camera2D children), signal connections, etc. I found Node.replace_by() method https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-method-replace-by which looks quite promising because it moves the children across, preserves signal connections, etc. but one of the other things it copies from the old to the new node is the scene it is an instance of:

https://github.com/godotengine/godot/blob/76addfff12b4dd1b8258ee9f14d4d8e03647cbf0/scene/main/node.cpp#L3273

which is exactly what we do not want!

[wjt]

I found a bug report about this:

• Replacing a node with an instantiated scene via replace_by causes the replacement to lose scene_file_path godotengine/godot#86314

and the workaround is simple: set scene_file_path back again...

[wjt]

Sadly the editor is now crashing so there is more to debug here. I will bash my head against this a little longer tomorrow before giving up.

[wjt]

With Godot 4.6, plus #1895 to work around a known issue in Godot, this seems to work better. I will pick it back up at some point.