GlassBricks / typed-factorio

Complete and featureful Typescript defintions for the Factorio modding API
MIT License
33 stars 3 forks source link

Subtypes for `LuaEntity` that follow the "Can only be used if this is <Prototype>" documentation #14

Closed aSemy closed 1 year ago

aSemy commented 2 years ago

Hi, I've got another request that might or might not fit in with the excellent work so far.

Some properties and functions in LuaEntity are documented as only being available for certain prototypes. For example, amount, initial_amount, and deplete() are only available when the prototype is ResourceEntity.

I'm asking because I want to find all entities of a specific type (resources, trees, buildings, biters...) in a chunk, and map each to a JSON object. I'd find that easier to do if I could do a type-check and then only the relevant fields are available, rather than going back and forth to verify it manually. Of course it's not a significant problem - but I wanted to make a request in case you think it's workable.

I think for backwards compatibility reasons LuaEntity should keep the 'Can only be used if' properties - but can they be changed to be optional? E.g. amount: uint -> amount: uint | undefined and deplete(): void -> deplete?():void

Usage demo

Here's a quick demo showing how the code would work after the changes

function handleLuaEntity(entity: TypedLuaEntity) {

  if (entity.entity_type == "resource") {
    handleResourceEntity(
        entity.amount, // no error
    )

    entity.deplete() // no error
  }
  if (entity.entity_type == "radar") {
    handleRadarEntity(
        entity.radar_scan_progress, // doesn't cause error: Type 'undefined' is not assignable to
                                    // type 'number'.
    )

    entity.deplete() // error: Cannot invoke an object which is possibly 'undefined'.
  }
}

function handleResourceEntity(amount: uint) {
  // ...
}

function handleRadarEntity(radar_scan_progress: float) {
  // ...
}

Library changes demo

And here's a quick demo of the library changes, in case this is more illuminating.

TypedLuaEntity would be a type-union of all the prototype names that are in the the documentation. I count 45 distinct "Can only be used if this is" references in the LuaEntity docs.

// union of all LuaEntity subtypes (can be scraped from the docs?)
type TypedLuaEntity =
    | ResourceLuaEntity
    | RadarLuaEntity

interface LuaEntity {

  // ...

  // change these properties and function to be marked as optional - override them in the subtypes
  deplete?(): void

  amount: uint  | undefined
  radar_scan_progress: float | undefined
}

interface ResourceLuaEntity extends LuaEntity {
  entity_type: "resource" // required for discriminating TypedLuaEntity

  deplete(): void 

  amount: uint 
}

interface RadarLuaEntity extends LuaEntity {
  entity_type: "radar" // required for discriminating TypedLuaEntity

  radar_scan_progress: float
}
GlassBricks commented 2 years ago

I consider doing something like this, however I found several major problems:

Instead, the best I could find was to have separate specialized subclasses, and let users do this typing more "manually". For example, with LuaEntity, you could use BaseEntity generally (which contains only properties shared by all types), and subclass types like RadarEntity where appropriate.

Additionally, typescript allows to make your own type narrowing functions:

function isRadarEntity(entity: BaseEntity): entity is RadarEntity {
    return entity.type === "radar"
}
function isEntityWithHealth(entity: BaseEntity): entity is EntityWithHealthEntity {
    return entity.is_entity_with_health
}
if(isRadarEntity(entity)) {
   // do something with entity.radar_scan_progress...
}

This can't be done automatically as this uses information beyond what can be gained from the json api docs. Additionally, this is is runtime code, which is beyond the scope of this project. Perhaps in the future we could make a separate project (a TSTL library) with a bunch of utility functions like above, defined manually.

It would be helpful to add this info to the documentation; I'll do that soon.

aSemy commented 2 years ago

Ah I see, the subclasses I proposed are already in the Typed-Factorio library! I didn't realise.

(Aside: It's quite hard to 'discover' these parts of the library because the files like classes.d.ts are so long - it would be easier to see what's available if they were split up into more specific files, like if entity interfaces like ResourceEntity were in a separate entity-classes.d.ts or something)

There is (currently) no way to discriminate between subclasses using a discriminated union. Your example above doesn't work, as entity_type isn't a property that exists, and we can't add it in (keep in mind these types describe an api, but don't define . entity.entity_type === "..." is code run at runtime and it would crash.

Okay, clear, no new fields. But type exists - is possible to set it to be a string literal type on the 'subclasses', e.g.

interface ResourceEntity {
  type: "resource"

  // ...
}

LuaEntity's (and other classes with "Can only be used with ...") often don't split cleanly into distinct subclasses; e.g. an entity can be both an AssemblingMachine and an EntityWithHealth, and have properties from both.

Okay yes, thanks for explaining. I'm not sure I quite understand though. Would it help if instead of LuaEntity being a super class, it an intersection of each 'subclass`?

type LuaEntity2 = LuaControl & BaseEntity
    & Partial<ResourceEntity>
    & Partial<RadarEntity>
    // ...

function asd(e: LuaEntity2) {
  const resourceE = e as ResourceEntity
  resourceE.deplete()
}
GlassBricks commented 2 years ago

Okay, clear, no new fields. But type exists - is possible to set it to be a string literal type on the 'subclasses', e.g.

This might be possible in some cases, however, it would need to be manually defined for every class and subclass and be difficult to get correct. Additionally, not all subclasses directly map to a property. For now, I don't think it's worth the effort to do this, hence the eventual compromise currently implemented.

Okay yes, thanks for explaining. I'm not sure I quite understand though. Would it help if instead of LuaEntity being a super class, it an intersection of each 'subclass`?

I don't think using Partial<Every subclass> does what we want here; it makes all properties optional, which means all properties now may take undefined. However, the runtime behavior for using a property with not appropriate subclass is usually to crash, and not read as nil (undefined). To actually get an error, BaseEntity (where instead property doesn't exist) should be used, and your example above works.