Allow compile to take custom compilation steps

[stargazing-dino]

first off, I'm not a fan of this PR as I think differentiating too far original C# implementation isn't the best idea.

Nevertheless I would love a way to add my own compile steps so i think it's worth discussing this or other ways to make it possible.

This changeset essentially adds an extra argument to compile allowing custom steps (they're added to the end of the default steps).

My first go at this, I tried to add the custom steps inside the Compiler. Although ideal that did not workout great since it required all sorts of bounds on the functions pointers. I'm not the best at rust so it quickly blew up in my face.

Definitely would be nice to hear it's possible though.

[janhohenheim]

@stargazing-dino I had the same thought while writing the code. Good idea!
For backwards and C# compatibility, I suggest adding a new function instead: compile_with_custom_steps.

[stargazing-dino]

Ooohh yeah, that'd work perfectly I think. I'll see if I can't do that this weekend.

My use case: I have a naive implementation of a node analyzer that builds a graph using petgraph and checks for unreachable nodes/recursion and such using CompiledProgramAnalyser but I think it'd be a lot easier to do at the tree level than the instruction one. (note, it's nothing like the fancy z3 stuff jon manning was doing but still :P)

Happy to contribute it once it's working

[janhohenheim]

@stargazing-dino that's a really cool idea! I'd love to include it in the crate :)

[stargazing-dino]

@janhohenheim Mind if I get your advice?

I got custom functions looking like this:

    /// Compiles the Yarn files previously added into a [`Compilation`].
    pub fn compile(&self) -> Result<Compilation> {
        run_compilation::compile(self, Self::default_compiler_steps())
    }

    /// Compiles the Yarn files previously added into a [`Compilation`] with custom compilation steps.
    pub fn compile_with_custom_steps(
        &self,
        custom_steps: Vec<&CompilationStep>,
    ) -> Result<Compilation> {
        run_compilation::compile(self, custom_steps)
    }

    /// The default compilation steps that the compiler will use.
    pub fn default_compiler_steps() -> Vec<&'static CompilationStep> {
        vec![
            &register_initial_variables,
            // etc.
        ]
    }

For that, I made all compiler_steps pub. I also had to pub the CompilationStep and anything around that.

I then got a simple test going that adds a line to the string table:

#[test]
fn test_compile_with_custom_steps() {
    let source = "
        <<declare $int = 42>>
        <<declare $str = \"Hello\">>
        <<declare $bool = true>>
    ";

    let custom_steps: Vec<&dyn Fn(CompilationIntermediate) -> CompilationIntermediate> = vec![
        // ... all other steps here
        &|state| {
            // Custom step: Update the string table
            let mut string_table = state.string_table;
            string_table.insert(
                "custom_line_id".into(),
                StringInfo {
                    text: "Custom line".to_string(),
                    node_name: "Start".to_string(),
                    line_number: 1,
                    file_name: "<input>".to_string(),
                    is_implicit_tag: false,
                    metadata: vec![],
                },
            );
            CompilationIntermediate {
                string_table,
                ..state
            }
        },
    ];

    let result = Compiler::from_test_source(source)
        .compile_with_custom_steps(custom_steps)
        .unwrap();

    // Check that the custom line is present in the string table
    assert!(result.string_table.contains_key(&"custom_line_id".into()));
    assert_eq!(
        result.string_table.get(&"custom_line_id".into()).unwrap(),
        &StringInfo {
            text: "Custom line".to_string(),
            node_name: "Start".to_string(),
            line_number: 1,
            file_name: "<input>".to_string(),
            is_implicit_tag: false,
            metadata: vec![],
        }
    );
}

Then I got sidetracked and thought well, there's not much use in that unless you have visitors...

and so took a crack at that and made a custom step that had it's own Visitor that counts the number of strings.

struct StringLiteralCounter {
    pub count: usize,
    _dummy: (),
}

impl StringLiteralCounter {
    pub fn new() -> Self {
        Self {
            count: 0,
            _dummy: (),
        }
    }
}

impl<'input> ParseTreeVisitorCompat<'input> for StringLiteralCounter {
    type Node = YarnSpinnerParserContextType;
    type Return = ();

    fn temp_result(&mut self) -> &mut Self::Return {
        &mut self._dummy
    }
}

impl<'input> YarnSpinnerParserVisitorCompat<'input> for StringLiteralCounter {
    fn visit_valueString(&mut self, _ctx: &ValueStringContext<'input>) -> Self::Return {
        self.count += 1;
    }
}

pub fn count_string_literals(mut state: CompilationIntermediate) -> CompilationIntermediate {
    let mut string_literal_count = 0;
    for file in &state.parsed_files {
        let mut visitor = StringLiteralCounter::new();
        visitor.visit(file.tree.as_ref());
        string_literal_count += visitor.count;
    }
    // state.string_literal = string_literal_count; // <-- Nope
    state
}

but the problem is that CompilationIntermediate is not extendable. We'd likely need to make it a trait so the user can make their own allowing for whatever metadata they want.

Anyways, what do you think about the

1. Scope of this PR. Is it too large? Should I break it up?
2. How pub should we go? I first made a test and only pubified things needed for it to compile but I don't know what's common.
3. Thoughts on making the CompilationIntermediate a trait?

[janhohenheim]

Hey, sorry for not looking into your query yet. I was really busy the last weeks and will probably still be for another week or so. I'll check it out in a more in-depth fashion then.
From a quick glance:

• Smaller PRs are always good, but if you don't feel like it, this one is still fine for me
• Try to keep things only pub(crate). pub types will be something that other devs will depend on, so we won't be able to refactor them later without breaking people's code. I haven't looked at the specific types in question yet, so I'll comment on them later. In general, the current pubs are oriented around what is public in the C# code.
• Sounds like a good solution. I'm also fine with storing Box<dyn CompilationIntermediate>, since this would definitely not be a performance bottleneck for us.

On another note, I'll make a release with your other merged PRs when Bevy 0.14 lands, if that is fine for you :)

[stargazing-dino]

Sounds good :D

Yeah, I'll likely start with the Box<dyn CompilationIntermediate> this weekend as it's own PR since it's largely self-contained

[janhohenheim]

@stargazing-dino I've got a bit more time now. Anything I can do to help with this PR? :)