Fix parallel task join orchestrator hard-resets its children.

[metaphore]

The Parallel task's Join orchestrator calls reset() on its children when completed. That renders them unusable for any further use within the tree. Looks like it's a simple mistake of calling reset() (pool-related hard reset for task instances) instead of resetTask() (soft "restart" so the task can be run again).

Here's a simple example that demonstrates the bug.

public class BTreeParallelJoinTest extends ApplicationAdapter {

    private BehaviorTree bTree;

    @Override
    public void create() {
        super.create();

        bTree = new BehaviorTree(
                new Repeat(ConstantIntegerDistribution.NEGATIVE_ONE, // Repeat forever.
                        new Parallel(Parallel.Orchestrator.Join,
                                new AlwaysFail( // Any decorator will do here.
                                        new Success()
                                )
                        )
                )
        );
    }

    @Override
    public void render() {
        super.render();
        bTree.step();
    }
}

The Repeat task crashes on the second iteration at the AlwaysFail decorator as its child was nullified by the reset() call:

Exception in thread "main" java.lang.NullPointerException: Cannot read field "status" because "this.child" is null
	at com.badlogic.gdx.ai.btree.Decorator.run(Decorator.java:65)
	at com.badlogic.gdx.ai.btree.branch.Parallel$Orchestrator$2.execute(Parallel.java:215)
	at com.badlogic.gdx.ai.btree.branch.Parallel.run(Parallel.java:123)
	at com.badlogic.gdx.ai.btree.LoopDecorator.run(LoopDecorator.java:56)
	at com.badlogic.gdx.ai.btree.BehaviorTree.step(BehaviorTree.java:124)
	...

[jonasbb]

It is possible to write code that does what you want. From a code perspective, the best place to add it would be the serde_derive macro, since that would allow placing the visit_map and visit_str functions next to each other, without any wrappers. You can write your own Deserialize implementation and then delegate to derive generated code using MapAccessDeserializer. That deserialize implementation would be possible to be generated by a derive macro.

With a small modification, it should be possible to have that as a derive macro. Generate a RegexWrapperRemote as an exact copy of RegexWrapper, apply the #[serde(remote="...")] to it, and use it in the Deserialize implementation.

However, I am not sure if this StringOrStruct derive is that important overall.

---

/*
[dependencies]
serde.version = "*"
serde.features = ["derive"]
serde_json = "*"
*/

use serde::Deserialize;
use serde::Deserializer;
use std::str::FromStr;

#[derive(Debug, Deserialize)]
#[serde(remote = "Self")]
struct RegexWrapper {
    pattern: String,
}

impl FromStr for RegexWrapper {
    type Err = String;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        Ok(Self {
            pattern: s.to_string(),
        })
    }
}

impl<'de> Deserialize<'de> for RegexWrapper {
    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
        struct V;

        impl<'de> serde::de::Visitor<'de> for V {
            type Value = RegexWrapper;

            fn expecting(&self, _f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
                todo!()
            }

            fn visit_str<E: serde::de::Error>(self, s: &str) -> Result<Self::Value, E> {
                FromStr::from_str(s).map_err(|e| E::custom(e))
            }

            fn visit_map<M: serde::de::MapAccess<'de>>(
                self,
                map: M,
            ) -> Result<Self::Value, M::Error> {
                RegexWrapper::deserialize(serde::de::value::MapAccessDeserializer::new(map))
            }
        }

        deserializer.deserialize_any(V)
    }
}

#[derive(Debug, Deserialize)]
struct ThingThatContainsRegex {
    blah: RegexWrapper,
}

fn main() {
    let _ = dbg!(serde_json::from_str::<ThingThatContainsRegex>(
        r#"{"blah": "foobar"}"#
    ));
    let _ = dbg!(serde_json::from_str::<ThingThatContainsRegex>(
        r#"{"blah": {
            "pattern": "foobar"
        }}"#
    ));
}

[Scripter17]

Finally got around to trying this and, uh, what?

How does this not cause a "trait implemented twice" error, or any error for that matter. How does #[serde(remote = "Self")] not explode into a stack overflow? Is this supposed to work or is it just a consequence of weird implementation details? How the hell does this just magically work???

And perhaps most importantly and answerably, is there a way to make the serialize derive macro not break with this? I do also need it to work with enums if that matters.

Oh, by the way, thank you so much. I was not ready to handle writing a deserializer for HashMap<String, OtherTypeWithSameFromStrThing> and this is a lifesaver.

[jonasbb]

The remote implementation does not create an impl Deserialize. Instead it adds a deserialize directly to the RegexWrapper struct. That means if you write RegexWrapper::deserialize you call the inherent function first and you need additional qualification if you want to call the deserialize function of the impl Deserialize for RegexWrapper. The impl Deserialize just calls the inherent function, avoiding any recursion issues.

The Serialize will "break" and there is no way to avoid it, as it will now only create an inherent function. You can create a minimal impl Serialize which just forwards to that function.

This remote = "Self" is a special case, but it can come in handy if you want to use the generated implementation but adjust them a bit. You can use cargo expand (available on the playground) to see the code after macro expansion.