Skip to content
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

Blueprint compilation support #54

Merged
merged 11 commits into from
Apr 1, 2024
Merged

Blueprint compilation support #54

merged 11 commits into from
Apr 1, 2024

Conversation

swazrgb
Copy link
Collaborator

@swazrgb swazrgb commented Mar 15, 2024

Supports compilation of exported blueprints, and nesting blueprints inside of behaviors. Behaviors can also be nested inside of blueprints.

Please see the tests/blueprint_with_behavior.ts file for a showcase of the supported features.

scripts/geninstr.ts Outdated Show resolved Hide resolved
assembler.ts Show resolved Hide resolved
type BlueprintArgs<I extends number, S extends number, M extends number, L extends number> = {
name?: string,
power?: boolean,
logistics?: {
Copy link
Collaborator Author

@swazrgb swazrgb Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not 100% sure about having these in a nested logistics block in the BlueprintArgs. I think the game blueprint JSON influenced my decision here, and these properties might as well all be top-level. What do you think?

So instead of the current definition, it could also be:

type BlueprintArgs<I extends number, S extends number, M extends number, L extends number> = {
  name?: string,
  power?: boolean,
  connected?: boolean,
  channels?: Array<number>,
  // etc
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either one seems fine with me. The do correlate with the logistics menu in the game, so having them nested makes sense.

Copy link
Collaborator Author

@swazrgb swazrgb Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the nesting, it turned out tedious to type without much benefit.

Copy link
Owner

@ribrdb ribrdb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for figuring this out. This is huge!

tests/__snapshots__/compile.test.ts.snap Show resolved Hide resolved
medium: [
component.roboticsAssembler([
[to("active1"), from("craft1")],
to("visual"),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this mean when it's not in an array?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's identical to an array containing a single entry.

A register can be linked to one or more other registers. to("visual") and [to("visual")] are identical, but the array syntax allows multiple links for that register.

I usually only work with a single link per register when making blueprints in the game, so I figured I'd make the array syntax optional here.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I think I misunderstood. I was thinking [to("active1"), from("craft1")] specified a single link craft1->active1. But you're saying that means [reg1->craft1, active1->reg1]? How does the user know how the registers should be ordered?

Copy link
Collaborator Author

@swazrgb swazrgb Mar 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that the user names the links they want to perform, then uses this name for both the from & to statements within that blueprint and those then get linked together. The names are entirely fictional and unrelated to any potential behavior controller parameter names, with the exception of "visual", "goto", "signal" and "store" which always relate to those registers[1])

to means the link is outgoing, from means link is incoming.

This example defines a Robotics Assembler which has three registers.

The first register has both a to link (active1) and a from link (craft1).

The to("active1") goes towards the behaviorController's from("active1"), and the from("craft1") comes from the behavior controller's to("craft1").

The order of [to("active1"), from("craft1")] does not matter, think of it as a Set.

In-game this would look like (when only defining the active1 & craft1 link):

image

When including the remaining two links for this robotics assembler (to("visual") and from("targetLocation")) it looks like this:

image

[1]: I am not sure about having visual, goto, signal and store be special names that always refer to those registers, since the user could also just explicitly define the links for those registers. E.g. these three are currently equivalent:

{
  goto: from("foo"),
  signal: to("foo")
}
{
  goto: to("signal")
}
{
  signal: from("goto")
}

There is some precedent for this since these registers are also available as globals, but the more I look at it, the less sure I am about it.


Anyway, I hope this makes sense. It's not super intuitive, but I couldn't come up with a better approach that lets the user easily define directional many-many relationships like the game allows.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. That could work, but we'd definitely need some documentation.
I wonder if we could simplify it so you don't need to name both ends of a connection. What if you could name, and specify only the names of registers that are linked to it?

blueprint.building2x1_2M_1({
    name: "Bot Factory",
    medium: [
        component.roboticsAssembler([
            {name:"active1", from:"craft1"},
            {name:"viz1"},
            {from:"targetLocation"},
        ]),
        component.roboticsAssembler([
            {name:"active2", from:"craft2"},
            {name:"viz2"},
            {from:"targetLocation"},
        ]),
    ],
    internal: [
        component.behaviorController(fnBotFactory, 
            [{name: "targetLocation"}, {from:"active1"}, {from:"active2"}, {name:"craft1"}, {name:"craft2"}]),
    ],
    visual: ["viz1", "viz2"]
});

I was also wondering if there's some way to use variable names instead of strings. If we had only a single array of components instead of multiple sizes we might be able to do something like this:

blueprint.building2x1_2M_1({
    name: "Bot Factory",
    components: [
        component.roboticsAssembler({slot:"medium"}),
        component.roboticsAssembler({slot:"medium"}),
        component.behaviorController(fnBotFactory /* maybe auto-select slot by default? */),
    ],
    links(frame, robots1, robots2, behavior) {
        frame.visual = [robots1[1], robots2[1]];
        robots1[0] = behavior[0];
        robots2[0] = behavior[0];
        behavior[1] = robots1[1];
        behavior[2] = robots[2]
        robots1[2] = behavior[3];
        robots2[2] = behavior[4];
    }
});

What do you think?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, I think what you've got is fine if you want to stick with that.

Copy link
Collaborator Author

@swazrgb swazrgb Mar 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you could name, and specify only the names of registers that are linked to it?

This is pretty close to what I was going for. The big difference is that using the keywords from and to allows the user to establish one-many, many-many, and one-one links with the same syntax. To be fair, I'm not sure how useful that acutally is, since I don't think many-many links are actually used that much in-game.

It feels odd to go from {name: "active", from: "craft2"} to [{name: "targetLocation"}, {from:"active1"}, {from:"active2"}]. In that case it'd feel more intuitive to me to do {name: "targetLocation", from: ["active1", "active2"]}

As an aside, not very relevant, but the from("foo") call produces a {from: "foo"} object, and the user is allowed to specify this object themselves. The from function is simply there as an utility, so in my implementation you can write {from: "foo"} in place of from("foo").

I was also wondering if there's some way to use variable names instead of strings

It's an interesting idea. My concern here is that it requires the user to count the indexes of their components, and update the links function if any component gets inserted before a previous component or re-ordered. This is not something that is actually ever required to produce a functional blueprint, but I find myself doing it anyway since I've developed a personal habit of having components in a specific order.

Another concern is that since it looks like regular javascript code, the user may expect to be able to use various syntax features such as loops, functions, the .length keyword, etc, and the complexity might increase rapidly. Of course most of that could be supported, but I'm not sure if the amount of work it would take is worth it.

But my main objection is that having to maintain the order of the links function argument would be annoying to do.

It would also probably require us to get rid of the compile-time information of how many slots of each component size the frame has, which can result in type errors. Of course this is also more of a nice-to-have, so it could be ditched.


One thing I definitely want to change is to get rid of the reserved names "goto", "visual", "signal" and "store". I'd still like these links to be implicitly possible if the user were to write e.g. to(goto) or from(goto). We already have declare var goto: Value; so this can be easily supported:

type BuiltinRegisterValue<N extends number> = Value & { regNum: N };
declare var goto: BuiltinRegisterValue<1>;
declare var store: BuiltinRegisterValue<2>;
declare var visual: BuiltinRegisterValue<3>;
declare var signal: BuiltinRegisterValue<4>;

Besides that I am considering several changes to the syntax. To compare, this is the reference in the syntax currently supported by this PR:

Reference Syntax

blueprint.building2x1_2M_1({
    name: "Bot Factory",
    medium: [
        component.roboticsAssembler([
            [to("active1"), from("craft1")],
            [to(visual), to(signal)],
            from(goto)
        ]),
    ],
    internal: [
        component.behaviorController(fnBotFactory, {
            targetLocation: to(goto),
            active1: from("active1"),
            craft1: to("craft1"),
        }),
    ]
});

Proposal 1

This gets rid of the from and to functions but keeps the general idea of naming the links. Instead the from/to's are defined in an object with the type of:

{
  to?: string | BuiltinRegisterValue | Array<string | BuiltinRegisterValue>,
  from?: string | BuiltinRegisterValue | Array<string | BuiltinRegisterValue>
}
blueprint.building2x1_2M_1({
    name: "Bot Factory",
    medium: [
        component.roboticsAssembler([
            {to: "active1", from: "craft1"},
            {to: [visual, signal]},
            {from: goto}
        ])
    ],
    internal: [
        component.behaviorController(fnBotFactory, {
            targetLocation: {to: goto},
            active1: {from: "active1"},
            craft1: {to: "craft1"},
        }),
    ]
});

Proposal 2

This is similar to proposal 1, but instead defines the keys name and from. name is similar to to, but unused names may exist, and name does not cause a link to happen on its own which requires the user to be more explicit.

{
  name?: string,
  to?: string | BuiltinRegisterValue | Array<string | BuiltinRegisterValue>
}
blueprint.building2x1_2M_1({
    name: "Bot Factory",
    medium: [
        component.roboticsAssembler([
            {name: "craft1", to: "active1"},
            {to: [visual, signal]},
            {name: "targetLocation"}
        ])
    ],
    internal: [
        component.behaviorController(fnBotFactory, {
            targetLocation: {to: goto},
            active1: {name: "active1"},
            // Can define unused names without an error
            active2: {name: "active2"},
            craft1: {to: "craft1"},
            craft2: {name: "craft2"},
        }),
    ],
    // Goto to link must be explicitly defined here
    goto: {to: "targetLocation"}
});

A nice-to-have here would be that behavior controllers their parameters could be named automatically. So this example could be reduced to:

blueprint.building2x1_2M_1({
    name: "Bot Factory",
    medium: [
        component.roboticsAssembler([
            {name: "craft1", to: "active1"},
            {to: [visual, signal]},
            {name: "targetLocation"}
        ])
    ],
    internal: [
        component.behaviorController(fnBotFactory, {
            targetLocation: {to: goto},
            craft1: {to: "craft1"},
        }),
    ],
    // Goto to link must be explicitly defined here
    goto: {to: "targetLocation"}
});

I'm now leaning towards proposal 2 since it allows unused names to exist (i.e. the user can comment out a "to" line and it does not produce an error, whereas an unused "from" would produce an error).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I implemented proposal 2.

tests/blueprint_with_behavior.ts Show resolved Hide resolved
tests/blueprint_with_behavior.ts Show resolved Hide resolved
type BlueprintArgs<I extends number, S extends number, M extends number, L extends number> = {
name?: string,
power?: boolean,
logistics?: {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either one seems fine with me. The do correlate with the logistics menu in the game, so having them nested makes sense.

scripts/geninstr.ts Outdated Show resolved Hide resolved
@swazrgb swazrgb force-pushed the blueprints branch 3 times, most recently from 29a92f8 to 86cae40 Compare March 25, 2024 01:11
@swazrgb swazrgb requested a review from ribrdb March 25, 2024 02:06
@swazrgb swazrgb force-pushed the blueprints branch 4 times, most recently from 59f5c9c to 0078158 Compare March 31, 2024 04:18
medium: [
component.roboticsAssembler([
{name: "craft1", to: "active1"},
{to: visual},
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you link from visual to a component's register?
Would that just be by specifying visual: {to: "foo"} at the top level of the behavior?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, together with {name: "foo"} for the targeted register.

compile.ts Outdated
@@ -55,20 +68,96 @@ function compileFile(f: ts.SourceFile): string {
throw new Error("sub ${subName} declared multiple times");
}
c.subs.set(subName, n);
} else if (ts.isVariableStatement(n)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is big enough it should probably move to its own function.

swazrgb added 10 commits April 1, 2024 04:13
* Separate sub & resolvedSub, since sub is necessary for callgraph calculation,
  and the Instructions array is visited multiple times if the same behavior
  or subroutine is included multiple times in the blueprint.

* Fix some bugs presumably related to IR refactor
This type was meant to translate a function parameters into an object,
but this turns out not to be possible in typescript. Function parameters
are represented as a named tuple, whose names are known by typescript
for documentation purposes, but not exposed to the type system.

see https://stackoverflow.com/a/69713319
Instead of naming the links via from/to, the registers are now
named, and 'to' links target these names.

Also use the global goto/visual/signal/store register variables
instead of letting their names be special
@swazrgb swazrgb merged commit b820dd3 into ribrdb:main Apr 1, 2024
3 checks passed
@swazrgb
Copy link
Collaborator Author

swazrgb commented Apr 1, 2024

@ribrdb I went ahead and merged this. If you did have any further feedback or run into issues feel free to let me know and I'll look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants