-
Notifications
You must be signed in to change notification settings - Fork 3
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
Conversation
scripts/geninstr.ts
Outdated
type BlueprintArgs<I extends number, S extends number, M extends number, L extends number> = { | ||
name?: string, | ||
power?: boolean, | ||
logistics?: { |
There was a problem hiding this comment.
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
}
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this 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/blueprint_with_behavior.ts
Outdated
medium: [ | ||
component.roboticsAssembler([ | ||
[to("active1"), from("craft1")], | ||
to("visual"), |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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):
When including the remaining two links for this robotics assembler (to("visual")
and from("targetLocation")
) it looks like this:
[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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 name
s 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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I implemented proposal 2.
scripts/geninstr.ts
Outdated
type BlueprintArgs<I extends number, S extends number, M extends number, L extends number> = { | ||
name?: string, | ||
power?: boolean, | ||
logistics?: { |
There was a problem hiding this comment.
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.
29a92f8
to
86cae40
Compare
59f5c9c
to
0078158
Compare
medium: [ | ||
component.roboticsAssembler([ | ||
{name: "craft1", to: "active1"}, | ||
{to: visual}, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)) { |
There was a problem hiding this comment.
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.
* 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
@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. |
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.