godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.14k stars 95 forks source link

Add node for apply script to parent #8934

Open mageowl opened 8 months ago

mageowl commented 8 months ago

Describe the project you are working on

Roguelike with collectable upgrades that modify the player controller.

Describe the problem or limitation you are having in your project

It is currently hard to have a script inject code into a different node's script, without writing your own boilerplate code. This is a very useful feature for games with upgrades that don't just change a number statistic, but make changes to the player controller, for example blocking any damage that is from fire enemies, or reducing knockback by 50%.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

It would be useful to be able to have a node called something like ScriptNode that when a script is attached to it, it adds it to the parent node, instead of applying it to itself. They could be stacked, and would apply in the order of the node hierarchy.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

For example, say the player has a function take_damage.

class_name Player extends CharacterBody2D

func take_damage(amount, type):
    hp -= type

Then, you pick up an upgrade that blocks all incoming fire damage. A ScriptNode is added to the player with the following script:

extends Player

func take_damage(amount, type):
    if type != "fire":
        super(amount, type)

The ScriptNode applies the upgrade script to the player, and anytime the take_damage function is called, if the damage type is not "fire", then the super is called, running the take_damage function in the Player script. If the damage is fire, nothing happens. You could then have ones for "lightning", "acid", and "drowning", and they could all be active at once, with each super calling the ScriptNode higher up on the list's take_damage function.

If this enhancement will not be used often, can it be worked around with a few lines of script?

You can use signals and functions like so:

# Player
signal mod_take_damage(args: Dictionary)

func take_damage(type, amount):
    var args = {"type": type, "amount": amount}
    mod_take_damage.emit(args)
    hp -= args.amount

# Upgrade
func take_damage(args): # Connected to the player's mod_take_damage signal
    if args.type == "fire":
        args.amount = 0

But it is more cumbersome, and there is no way to completely cancel the event.

Is there a reason why this should be core and not an add-on in the asset library?

This is extremely flexible and can be used in many different applications.

Mickeon commented 8 months ago

The whole structure of this code is kind of an anti-pattern. I'm not convinced adding a whole node that encourages this would benefit many. That's not to mention this ScriptNode's name and behavior is not really intuitive by itself.

There's many different ways to go about alternatives that are more "proper" while still keeping the code very dynamic, as is the main issue in the proposal. I'm not sure where to start.

mageowl commented 8 months ago

Yeah, I'm not sure about the name, it's mostly a placeholder. Right now, it takes a lot of boilerplate that feels like hacking to be able to apply these kind of modifiers to an object at runtime. I think that the problem still needs solving, if not in this fashion.

dalexeev commented 8 months ago

For me, the only reasonable option for injecting methods into a class is traits, but they have nothing to do with the issue. This is more of an architectural problem than a language problem. You can solve it in many ways, and some boilerplate is often inevitable.

But it is more cumbersome, and there is no way to completely cancel the event.

You could implement it like this:

# damage.gd
class_name Damage
extends RefCounted

enum Type {
    FIRE,
    SHOCK,
    ICE,
}

var type: Type

var amount: int:
    set(value):
        amount = maxi(0, value)

func _init(p_type: Type, p_amount: int) -> void:
    type = p_type
    amount = p_amount
# player.gd
func take_damage(damage: Damage) -> void:
    for modifier in _modifiers:
        if not modifier.take_damage(damage):
            return
    hp -= damage.amount
# shield.gd
func take_damage(damage: Damage) -> bool:
    if damage.type == Damage.FIRE:
        return false
    if damage.type == Damage.SHOCK:
        damage.amount -= 1
    return true
mageowl commented 8 months ago

I think that being able to apply traits at runtime to singular nodes, instead of all instances of a script would solve this issue.

dalexeev commented 8 months ago

No, traits do not imply runtime application and also do not support multi-adding of a method with the same name from different traits (in this case you must explicitly resolve the conflict).

The same applies to this proposal: it is not obvious in what order the modifiers should be called, whether a modifier can stop the chain, whether a modifier can call parent class method (that's why super in your example is not good, that would require a different keyword).

GDScript doesn't allow for this kind of function code patching at runtime, and you can't replace a method with a wrapper like in JavaScript (you could emulate this with Callables, but that's probably a bad idea).

Also, Node is not a base class. Yes, we have constructs for Node in the language (like @onready and $Node), but function is a more fundamental concept than Node.

Mickeon commented 8 months ago

I will repeat, there's many ways to go about this issue that are not just straightforward but more structurally sound. Looking up "Node as Components" may shed some light on how use Nodes very dynamically... ... If necessary. It's ok to make code simpler. Sometimes, this modularity is not necessary.

One other way to go about it, specifically tailored to the example in the proposal, is to make use of RefCounted/Resources. The goal is to make use of another object to swap out logic as desired.

Create some sort of "Defense" object with the following function:

func should_resist_damage(amount, type):
    return type == "fire"

Add a defense property to the Player script. When you collect a powerup, assign a new RefCounted with the script above attached. Then, the following bit of logic:

func take_damage(amount, type):
    if defense != null and defense.should_resist_damage(amount, type):
        return # Do nothing.

    hp -= type

This is not an uncommon pattern. It's a form of dependency injection. Perhaps it may look ugly because I do not know the full context, and as such I cannot suggest a whole set of names and methods for this class.