ribrdb / desynced-tools

Tools for working with behaviors and blueprints from Desynced.
MIT License
4 stars 3 forks source link

Support literal ids in typescript #35

Closed ribrdb closed 5 months ago

ribrdb commented 8 months ago

For example, say I want to set my signal to c_miner or c_miner@2; signal = "c_miner" gives a type error and doesn't emit anything if you ignore the error. Maybe we should expose set_reg as a function or method:

signal.set('c_miner', 2)

Or should Value have an id property?

signal.id = 'c_miner'

That could work for coords too:

goto.coord = {x: 150, y: 150}
ribrdb commented 8 months ago

@quisar do you have any suggestions for how this should work?

quisar commented 8 months ago

I do not have a very firm opinion. I like the idea of having properties, but I do not think it really solves the issue, as it makes more sense if we only use this to change part of the Value (either the id, the count, the coord, etc.) and so would map more to combineRegister We would still need a way to build a value from id and number. For this, we have a couple of issue:

  1. Make the type checker happy. Already today, writing a purely numerical function is possible but will get the compiler to complain, because using a number for a Value is incorrect.
  2. Doing something efficient. If we build some kind of syntax for constants, we will need to be careful not to have code generating intermediary variable with the constant, and then using that variable afterwards. We can probably solve this in the compiler itself by generating some specific type of const Value that we can expand each time they are used.
  3. Your set suggestion probably work, but would be very awkward to do something like:
    let c: Value;
    c.set('c_miner', 2);
    ...

    as I expect the compiler to complain that c has not been initialized.

ribrdb commented 6 months ago

Probably the best solution is to just make 'setReg' a function. Maybe we should call it something like makeValue?

let c = makeValue('c_miner', 2)
swazrgb commented 6 months ago

I think that's a good option.

One possible alternative is to let String & Number extend the interface defining all the methods, for example:

interface Value {
  type: "No Match" | "Item" | "Entity" | "Component" | "Tech" | "Value" | "Coord";

  // Methods go here
  fullHealth(): boolean;

}

// Use AnyValue for argument types, can use specific types for return values
// Value is not used for arguments or return types
declare function getIngredients(product: AnyValue): [ItemNum, ItemNum, ItemNum];

interface FrameNumImpl extends BaseValue {
  id: Frame;
  num: number;
}
type FrameNum = Frame | number | FrameNumImpl;

interface ItemNumImpl extends Value {
  id: string,
  num: number
}
type ItemNum = Item | number | ItemNumImpl;

interface Coord extends Array<number>, Value {}

// The magic happens here. All other types of AnyValue must also extend Value, like ItemNumImpl
interface String extends Value {}
interface Number extends Value {}

This should allow users to write code like this:

export function foo(v: AnyValue) {
    if(v.fullHealth()) {
        v = "c_alien_attack";
    } else {
        v = {
            id: "test",
            num: 5
        } as ItemNum;
    }

    switch(v.type) {
        case "Item":
            v = 10;
            break;
    }

    let [a,b,c] = getIngredients(v);
    if(a == "alien_artifact") {
        // Type checked   
    }

    if(a == "some_other_string") {
        // Intentional type error
    }
}

This way you can keep setReg hidden and the user can just do normal variable assignments without always having to call a function. But, generally, messing with the prototype of built-ins is discouraged. But it might be nice here since it's basically what the compiler is allowing anyways.

swazrgb commented 6 months ago

Although the benefit of a makeValue function is that multiple overrides can be provided, so the user doesn't have to cast manually.

        v = {
            id: "test",
            num: 5
        } as ItemNum;

Isn't very nice, and it'll likely be written a lot. So a utility function for ItemNum and FrameNum would be helpful.

declare function makeValue(a: Frame, b: number): FrameNum;
declare function makeValue(a: Item, b: number): ItemNum;

That would let users write:

signal = 5;
signal = "metalore";
signal = makeValue("metalore", 5);

If you are interested I'd be happy to take a stab at implementing this.

ribrdb commented 6 months ago

Yeah, that would be great!

swazrgb commented 6 months ago

https://github.com/ribrdb/desynced-tools/pull/44