-
-
Notifications
You must be signed in to change notification settings - Fork 240
match_class!
now supports dynamic dispatch (@ dyn Trait
pattern).
#1256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
match_class!
now supports dynamic dispatch
match_class!
now supports dynamic dispatchmatch_class!
now supports dynamic dispatch.
match_class!
now supports dynamic dispatch.match_class!
now supports dynamic dispatch (@ dyn Trait
pattern).
Can you please elaborate the broader use case -- why do we need this? The idea of traits and dynamic polymorphism is to abstract from concrete dispatching. I'm not sure if we should encourage more such "type checking" patterns. The use case is in-so-far different from class checking that Godot's API is designed in a way to handle different subclasses as one way of runtime dispatch (inputs, shapes, ...).
|
Oh, yeah! I was needing this such API when making the jam, so I have one concrete example. I'd say one of the main uses of try dynify is in signal receivers, where one only gets a Gd pointer to the base class. It allows for handling more than one possible action when that happens. I'd say collisions could very easily be where this would be most used, as in a game you can have different logic happening for every collision and it's sometimes better to implement it as a Rigid or Static, or even as Area, so it is a perfect place for dynamic dispatch since you cannot simply do virtual methods and call it a day. |
But when you have a trait, you no longer need to explicitly differentiate cases -- that's the whole point of polymorphism. So where's the need for Maybe provide a small motivating example that cannot be easily solved without it. |
Oh, yeah, exactly, but then you need to differentiate between traits.
Collisions based interactions. You want to have different collidable objects: Collectibles, obstacles, enemies, danger zones.
Obstacles, enemies and danger zones all do different things. For example, danger zones and enemies require to know when the player enters and leaves, obstacles only requires entering. Thus, different traits. The only way to make this all work without matching, that I can think of, is to make two traits, OnCollisionEntered and OnCollisionExited, the first two inherit Entered while the latter inherit both, then on entered one would try dynify the first, same for the on exited. But then the problem that arises is the fact that OnCollisionEntered and OnCollisionExited both need to accept the same parameters, which would pass unnecessary parameters around for most cases (item or danger zones don't need RID or shape indexes at all, for example). |
I don't agree with the implication. You can have one trait with
Why is passing unnecessary parameters a problem? Let the entities just access the parts they need. After all, if you did So far, this doesn't convince me and it seems like a design problem that might be solved differently. If you think otherwise, please come up with a minimal code example that shows the need for this feature (just 2-4 classes/traits). It might be easier to see with concrete code 🙂 |
Alright, I will give it some more thought.
What do you mean?
Not in the general case, but it could be designed that way by the user.
Is there talk about it currently? To try to tackle the problem sometime soon |
The current docs say:
For example: as a future optimization, it would be possible for us to build a graph of the class hierarchy ahead of time, check the object's dynamic type, and then dispatch in O(1) instead of current O(n). Not saying we should do it, but we have this option. With traits, this won't be possible. The object may implement multiple traits appearing in match arms, and none is objectively "better". |
What I meant with it is that maybe (haven't seen how the tree? of class hierarchy would be built so it may not be possible) We could also do a tree with trait hierarchy (by means of :), and enforce that the match can only be used on orthogonally implemented traits (no two traits in the match are implemented by any one struct) or clearly hierarchical traits (only trait and ONE of its supertraits is in the match statement). Requires a bit more work, but I think it could be done. In any case, we can also move this to match_dyn! { node,
trait_1 @ dyn Trait1 => (),
trait_2 @ dyn Trait2 => (),
node => {
match_class! { node,
node_2d @ Node2D => (),
node_3d @ Node3D => (),
}
}
} |
This is what callable is for: https://docs.godotengine.org/en/4.4/classes/class_callable.html. Your Entereteretables can return an Check Bob Nystorm (Game Programming Patterns guy, he is mentioned in godot docs) talk about employing patterns to manage such (and more!) problems https://www.youtube.com/watch?v=JxI3Eu5DPwE. I could be wrong, it is hard to reason about concrete problem without minimal example which shows the need for this feature 🤷 .
There is one core issue – traits in rust don't do that and as a user I would expect There are also cases in which compile-time hierarchy just doesn't work well and imposing it would be silly – and doing so only in one I believe that enforcing any kind of OOP-inspired hierarchy on users would steer them into using non-optimal, non-rusty patterns, which don't work too well and are hard to maintain. Consider this example – I have items which implement Bar and Baz (real world example would be Entity/WorldItem/Pickup/AiNode etc) and want to run Bar and Baz stuff with them. Two ways to do dynamic dispatch on runtime with trait objectstrait Bar {
fn bark(&self) {
println!("I am bar!");
}
}
trait Baz {
fn bazk(&self) {
println!("Bzzz bzzz");
}
}
#[derive(Eq, PartialEq)]
enum DomainType {
Bar,
Baz,
}
trait DomainMember {
fn domains(&self) -> &[DomainType] {
&[]
}
fn as_bar(&self) -> Option<& dyn Bar> {
None
}
fn as_baz(&self) -> Option<&dyn Baz> {
None
}
}
struct FooBar;
impl Bar for FooBar {}
impl DomainMember for FooBar {
fn domains(&self) -> &[DomainType] {
&[DomainType::Bar]
}
fn as_bar(& self) -> Option<&dyn Bar> {
Some(self)
}
}
struct FooBaz;
impl Baz for FooBaz {}
impl DomainMember for FooBaz {
fn domains(&self) -> &[DomainType] {
&[DomainType::Baz]
}
fn as_baz(&self) -> Option<&dyn Baz> {
Some(self)
}
}
struct FooBazBar;
impl Baz for FooBazBar {}
impl Bar for FooBazBar {}
impl DomainMember for FooBazBar {
fn domains(&self) -> &[DomainType] {
&[DomainType::Baz, DomainType::Bar]
}
fn as_baz(&self) -> Option<&dyn Baz> {
Some(self)
}
fn as_bar(&self) -> Option<&dyn Bar> {
Some(self)
}
}
fn do_something(things: &Vec<Box<dyn DomainMember>>) {
for thing in things {
let domains = thing.domains();
match (
domains.contains(&DomainType::Bar),
domains.contains(&DomainType::Baz),
) {
(true, true) => {
thing.as_bar().unwrap().bark();
thing.as_baz().unwrap().bazk();
}
(true, false) => {
thing.as_bar().unwrap().bark();
}
(false, true) => {
thing.as_baz().unwrap().bazk();
}
_ => {
unreachable!("dum dum")
}
}
}
}
fn do_it_differently(things: &Vec<Box<dyn DomainMember>>) {
for thing in things {
if let Some(bar) = thing.as_bar() {
bar.bark();
}
if let Some(baz) = thing.as_baz() {
baz.bazk();
}
}
}
fn main() {
let domain_members: Vec<Box<dyn DomainMember>> = vec![
Box::new(FooBar {}),
Box::new(FooBaz {}),
Box::new(FooBar {}),
Box::new(FooBazBar {}),
];
println!("Fairly weird way to do a dynamic dispatch:");
do_something(&domain_members);
println!("\nAnd now a bit more rusty way");
do_it_differently(&domain_members);
}
The point is – Trait represent Functionality implemented by given type, not any kind of the hierarchy; using them for hierarchy is just wrong (especially since rust doesn't provide specialization). |
My what? Wdym? I don't really understand what you mean.
Alright, then, no need to make it work like that, then optimizations can be done on match_class and we could move this to match_dyn such that the optimizations in one don't clash with the other. Or they could be mixed together and only optimize the match_class branches. Alright, example goes: struct Player {
health: usize,
max_health: usize,
enemy_hit_count: usize,
zone_hit_count: usize,
}
trait Damage {
pub fn do_damage(&self, health: &mut usize) {}
}
trait Character {
pub fn mut_health(&mut self) -> &mut usize;
pub fn take_damage<D: Damage>(&mut self, damager: &D) {
damager.do_damage(self.mut_health());
}
}
trait Enemy: Damage + Character {
fn collision_entered(&mut self, node: Gd<Node>);
}
impl Enemy for EnemyImplementor {
fn collision_entered(&mut self, node: Gd<Node>) {
if let Ok(damage_zone) = node.try_dynify::<dyn Damage>() {
self.take_damage(damage_zone.dyn_bind());
}
}
}
impl Character for Player {
pub fn mut_health(&mut self) -> &mut usize { &mut self.health }
}
impl Player {
fn collision_entered(&mut self, node: Gd<Node>) {
match_dyn! { node,
enemy @ dyn Enemy {
self.enemy_hit_count += 1;
self.take_damage(enemy.dyn_bind());
},
damage_zone @ dyn Damage {
self.zone_hit_count += 1;
self.take_damage(damage_zone.dyn_bind());
},
}
}
} Here player needs to collect statistics for some reason, and there's a clear difference between enemies and any damaging entity, so they are two different statistics that need to be tracked separately. |
Is this example reasonably useful? Should I make changes to separate the match_dyn from match_class? Keep them together but allow for different optimizations? Scratch the concept altogether? |
But the |
It was a minimal examples, but you could extend it to more things than that, others that don't do damage... Sure, there are many things you can do, such as implementing two traits that are the collider and the collided, then have many enums set in place, but that's much more machinery behind it, and you'd need the power of foresight. If the intention of DynGd is to only use one per function maybe we could do #[dyn_func] that takes DynGd in its signature and transforms it from #[dyn_func(..., default = Default::default)] // would accept panic (not specified), default (uses Default::default), default = func (where func is an expression that evaluates to impl Fn() -> ReturnType)
pub? fn func_name(&self, dyn_1: DynGd<DynNode, dyn DynTrait>, ...) -> ReturnType {
...
} to pub? fn _func_name(&self, dyn_1: DynGd<DynNode, dyn DynTrait>, ...) -> ReturnType {
...
}
#[func(...)]
pub? fn func_name(&self, dyn_1: Gd<DynNode>, ...) -> ReturnType {
let Ok(dyn_1) = dyn_1.try_dinify() else { return Default::default(); };
...
_func_name(&self, dyn_1, ...)
} |
Thanks a lot for the example! If I understand correctly, you want two things:
I would model this differently: struct Player {
health: usize,
max_health: usize,
enemy_hit_count: usize,
zone_hit_count: usize,
}
trait Damage {
fn amount(&self) -> usize; // no &mut
fn stat(&self) -> DamageStat; // Enemy or Zone
}
impl Player {
fn collision_entered(&mut self, node: Gd<Node>) {
if let Some(damage) = node.try_dynify::<dyn Damage>() {
self.hitpoints -= damage.amount()
match damage.stat() {
DamageStat::Enemy => { self.enemy_hit_count += 1; }
DamageStat::Zone => { self.zone_hit_count += 1; }
}
}
}
} I know it's just a minimal example, but it's important that it reflects some of the real challenges you're facing, and we're not entering a moving-goalposts situation where every time a solution is suggested, a "yes but what if" answer comes 😉 Also, some of that code looks rather overengineered. Not every relation needs to be modeled as a trait. Code like this in particular: pub fn take_damage<D: Damage>(&mut self, damager: &D) {
damager.do_damage(self.mut_health());
} is rather involved, and would likely not be written like that outside Rust. It's a form of the visitor pattern, but is it needed at this complexity level? Furthermore, you already run into the OOP hierarchy problem: trait Damage {}
trait Character {}
trait Enemy: Damage + Character {} This won't scale. You'll need a Maybe a tip: think how you would model this entirely without traits. It's definitely possible, may need more differentiation in some places and less boilerplate in others. Then compare the approaches, and try to take the best of both worlds. |
I still would argue that in this case event-based approach (or command object based approach which can be neatly provided by Godot's Callable) is better than compile time hierarchy (and still allows for compile-time validty with rust enums!). In this example any Entrancer generates set of some effects, which are being resolved by the consumer. This approach allows for much more flexibility – if you change something in your domain you just need to create/append/modify your effects, which is not that easy to do with compile-time hierarchy. example of action-based approach to such issuetrait Damage {
fn do_damage(&self, health: &mut i64);
}
trait Character {
fn mut_health(&mut self) -> &mut i64;
fn take_damage<Visitor: Damage + ?Sized>(&mut self, damager: &Visitor) {
damager.do_damage(self.mut_health());
}
fn resolve_effect(&mut self, effect: CollisionEffect);
}
trait Entrancer {
fn entrance_effects(&self) -> Vec<CollisionEffect>;
}
enum Stat {
EnemyHitCount,
ZoneHitCount,
PickupsCount
}
enum CollisionEffect<'a> {
TakeDamage(&'a dyn Damage),
ChangeStat { stat: Stat },
LogMessage { message: String },
AppendWithSpikes { amount: i64 },
}
#[derive(Default)]
struct StatsCounter {
enemy_hits: i64,
zone_hits: i64,
pickups: i64,
}
impl Index<Stat> for StatsCounter {
type Output = i64;
fn index(&self, index: Stat) -> &Self::Output {
match index {
Stat::EnemyHitCount => &self.enemy_hits,
Stat::ZoneHitCount => &self.zone_hits,
Stat::PickupsCount => &self.pickups,
}
}
}
impl IndexMut<Stat> for StatsCounter {
fn index_mut(&mut self, index: Stat) -> &mut Self::Output {
match index {
Stat::EnemyHitCount => &mut self.enemy_hits,
Stat::ZoneHitCount => &mut self.zone_hits,
Stat::PickupsCount => &mut self.pickups,
}
}
}
struct Enemy {
health: i64,
}
impl Damage for Enemy {
fn do_damage(&self, health: &mut i64) {
*health -= 2;
}
}
impl Entrancer for Enemy {
fn entrance_effects(&self) -> Vec<CollisionEffect> {
vec![
CollisionEffect::ChangeStat { stat: Stat::EnemyHitCount },
CollisionEffect::TakeDamage(self)
]
}
}
impl Character for Enemy {
fn mut_health(&mut self) -> &mut i64 {
&mut self.health
}
fn resolve_effect(&mut self, effect: CollisionEffect) {
match effect {
CollisionEffect::TakeDamage(damage) => {
self.take_damage(damage);
}
CollisionEffect::LogMessage { message } => {
println!("new message from enemy: {message}")
}
// Discard any other effect.
_ => {}
}
}
}
struct SpikeTrap {}
impl Damage for SpikeTrap {
fn do_damage(&self, health: &mut i64) {
*health -= 1;
}
}
impl Entrancer for SpikeTrap {
fn entrance_effects(&self) -> Vec<CollisionEffect> {
vec![
CollisionEffect::TakeDamage(self),
CollisionEffect::ChangeStat {stat: Stat::ZoneHitCount }
]
}
}
struct DamagePickup {}
impl Entrancer for DamagePickup {
fn entrance_effects(&self) -> Vec<CollisionEffect> {
vec![
CollisionEffect::ChangeStat { stat: Stat::PickupsCount },
CollisionEffect::AppendWithSpikes { amount: 1},
CollisionEffect::LogMessage { message: String::from("Damage Pickup has been picked up!") }
]
}
}
struct PlayerSpikes {
damage: i64
}
impl Damage for PlayerSpikes {
fn do_damage(&self, health: &mut i64) {
*health -= self.damage;
}
}
struct Player {
health: i64,
max_health: i64,
damage_component: Option<PlayerSpikes>,
stats: StatsCounter
}
impl Character for Player {
fn mut_health(&mut self) -> &mut i64 { &mut self.health }
fn resolve_effect(&mut self, effect: CollisionEffect) {
match effect {
CollisionEffect::TakeDamage( damager ) => {self.take_damage(damager)}
CollisionEffect::ChangeStat { stat } => self.stats[stat] += 1,
CollisionEffect::LogMessage { message } => println!("{message}"),
CollisionEffect::AppendWithSpikes {amount} => {
if let Some(damage_component) = self.damage_component.as_mut() {
*&mut damage_component.damage += amount;
} else {
self.damage_component = Some(PlayerSpikes { damage: amount });
}
}
}
}
}
impl Entrancer for Player {
fn entrance_effects(&self) -> Vec<CollisionEffect> {
let mut effects = Vec::new();
if let Some(damage) = self.damage_component.as_ref() {
effects.push(CollisionEffect::TakeDamage(damage));
}
effects
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_gameplay_scenario() {
let mut player = Player {
health: 20,
max_health: 40,
damage_component: None,
stats: Default::default(),
};
let mut enemy = Enemy {
health: 10,
};
// Player collides with the enemy and takes damage.
for effect in enemy.entrance_effects() {
player.resolve_effect(effect);
}
assert_eq!(player.health, 18);
assert_eq!(player.stats.enemy_hits, 1);
// Enemy collides with the player and nothing happens.
for effect in player.entrance_effects() {
enemy.resolve_effect(effect);
}
assert_eq!(enemy.health, 10);
let trap = SpikeTrap {};
for effect in trap.entrance_effects() {
player.resolve_effect(effect);
}
assert_eq!(player.health, 17);
assert_eq!(player.stats.zone_hits, 1);
let damage_pickup = DamagePickup {};
// Let's pick up 2 damage powerups.
for effect in damage_pickup.entrance_effects() {
player.resolve_effect(effect);
}
for effect in damage_pickup.entrance_effects() {
player.resolve_effect(effect);
}
assert_eq!(player.stats.pickups, 2);
// Player collides with the enemy and takes damage.
for effect in enemy.entrance_effects() {
player.resolve_effect(effect);
}
assert_eq!(player.health, 15);
assert_eq!(player.stats.enemy_hits, 2);
// Enemy collides with the player – this time they take the damage!
for effect in player.entrance_effects() {
enemy.resolve_effect(effect);
}
assert_eq!(enemy.health, 8);
}
} I'm also not a huge fan of visitor pattern 😅, but I left it in for the sake of the example. The problem with compile-time hierarchy is that requirements change and not everything can be neatly encapsulated by it (i.e. it doesn't scale well) – once again, I encourage you to watch linked Bob Nystorm talk about it. Obviously the "real" implementation would differ based on your requirements. |
Complements #1255
Adds try_dynify branches to match class, preparing for when the PR is merged.