lampe-games / godot-open-rts

Open Source RTS game made in Godot 4
https://lampe-games.itch.io/open-rts
MIT License
593 stars 79 forks source link

Refer units by type instead of gd file #105

Open lmorao opened 7 months ago

lmorao commented 7 months ago

Hello,

there's a lot of paths used in the files, like for example in res://source/match/units/actions/CollectingResourcesSequentially.gd

const Worker = preload("res://source/match/units/Worker.gd")
static func is_applicable(source_unit, target_unit):
    return (
        (source_unit is Worker and target_unit is ResourceUnit)
        or (source_unit is Worker and target_unit is CommandCenter and target_unit.is_constructed())
    )

As an alternative, I see that the base unit class has a type setting. My question is, won't it be easier in general to use unit.type == "Worker" instead of doing unit is Worker?

This would simplify the example above to not require the const

static func is_applicable(source_unit, target_unit):
    return (
        (source_unit.type == "Worker" and target_unit is ResourceUnit)
        or (source_unit.type == "Worker" and target_unit is CommandCenter and target_unit.is_constructed())
    )

The type function doesn't exist on structures, but it would be easy to copy paste it there so that this can be extended to structures as well.

Is there a drawback to doing this? Right now the current drawback is that we can't move files around, and also if I want a second type of worker, I need to go to all the files that use the Worker and add another worker reference path, and change the logic to include the other worker type for all the checks.

Conversely with this change, I could make a new worker, "hardcode" the type to "Worker" and it would just work.

Regards, Luís

Scony commented 7 months ago

Hi, the unit.type was added fairly recently. Anyway, you're probably right that it could be used instead of precise type checking. The drawback is ofc. that it's prone to name collisions if you have 2 different Worker scenes in different parts of the filesystem tree. As for reusability, IMO the type should not be used. The type should be used to uniquely identify units so that e.g. you can distinguish between 2 different types of workers. If you'd like to introduce some functionality common to all worker types you should rather add each worker to group called e.g. workers and then check if unit belongs to that group.