Full TypeScript rewrite. Best type defs by zardoy · Pull Request #651 · PrismarineJS/flying-squid

zardoy · 2024-01-17T16:18:22Z

closes #323

~~Still a lot of commits to pick, but decided to open early to run a few checks (like jsdoc description & remove docs, generator).~~ awaiting feedback

I have published module for testing so you can see the final result if interested (including docs URL): http://npmjs.com/package/@zardoy/flying-squid

(cherry picked from commit 3cbf1a4)

(cherry picked from commit 1acb152) with a lot of manual fixes

zardoy · 2024-01-17T16:40:46Z

    const count = opt.count !== undefined
      ? opt.count
-      : (type === 'all' || type === 'entity' ? serv.entities.length : 1)
+      : (type === 'all' || type === 'entity' ? Object.keys(serv.entities).length : 1)


zardoy · 2024-01-17T16:48:06Z

+    "_writeArray": (packetName: any, packetFields: any, players: any) => any
+    "_writeNearby": (packetName: any, packetFields: any, loc: any) => any
+    "getNearby": ({ world, position, radius }: { world: any; position: any; radius?: number | undefined }) => any
+    "getNearbyEntities": ({ world, position, radius }: { world: any; position: any; radius?: number | undefined }) => any[]


Need to remove destructure from types.

zardoy · 2024-01-17T16:51:05Z

  entity.sendEffect = (effectId, { amplifier = 0, duration = 30 * 20, particles = true, whitelist, blacklist = [] } = {}) => {
    if (!whitelist) whitelist = serv.getNearby(entity)
-    if (entity.type === 'player' && [1].indexOf(effectId) !== -1) entity.sendAbilities()
+    if (entity.type === 'player' && [1].indexOf(effectId) !== -1) (entity as Player).sendAbilities()


I could turn entity into union of "common entity" and "player" so checks like entity.type === player would narrow the type to Player. But it's not possible since we re using interfaces for per-module extension and not types. So it would be easier to just mention this trick

zardoy · 2024-01-17T17:25:03Z

    const block = await player.world.getBlock(referencePosition)
    block.position = referencePosition
+    //@ts-ignore TODO
    block.direction = direction


Not sure why this pattern is widely used across prismarine projects, imo it shouldn't as it makes debugging hard

(cherry picked from commit 9748f4f)

remove almost all require's, make mcData typed (expose), make types ready for publishing! (cherry picked from commit e4be9d8)

add block renames (cherry picked from commit b9a2813)

(cherry picked from commit f324dc4)

…rnal remove api.md for now fix gentypes (cherry picked from commit dd3253e)

…r overworld (cherry picked from commit 8297df2)

(cherry picked from commit 9d72f86)

(cherry picked from commit 46edddc)

(cherry picked from commit ba37e8d)

(cherry picked from commit b81e264)

(cherry picked from commit 692f062)

socket-security · 2024-01-17T19:06:43Z

No top level dependency changes detected. Learn more about Socket for GitHub ↗︎

zardoy · 2024-01-17T19:12:50Z

almost done, I picked everything. will review it once again and mark it as ready.

I tried ts-standard instead of standard. While it uses some useful rules like @typescript-eslint/no-floating-promises it also includes too strict rules like @typescript-eslint/explicit-function-return-type that require you to write more code. Overall ts-standard ruleset looks good to me so if you want I can fix all errors.

rom1504 · 2024-01-17T19:13:37Z

Please fix tests

rom1504 · 2024-01-17T19:34:32Z

I see you deleted the doc. Can you show what a generated doc would look like ?

rom1504 · 2024-01-17T19:35:36Z

Overall doesn't look worse than before.

But also it's not super clear to me what we gain here. Just adds a bunch of syntactic complexity and maybe a small amount of type safeness.

What do you think @extremeheat ?

rom1504 · 2024-01-17T19:37:18Z

https://github.com/zardoy/space-squid/tree/ts-master/src/lib this is the part that may maybe benefit from TS but somehow you didn't convert it. Any reason?

zardoy · 2024-01-17T20:17:10Z

I see you deleted the doc. Can you show what a generated doc would look like ?

Yes give me a moment. Also let's discuss it in the last step, not the biggest issue here imo.

Just adds a bunch of syntactic complexity

Let's revert things you don't like here. I just want basic IntelliSense for serv/entity/player (and typings!).

small amount of type safeness.

Yes I fixed a small amount of bugs that types helped me to catch. But in the current state the codebase is almost fully typed so referencing would allow you to prototype faster. Or did you mean something else?

zardoy/space-squid@ts-master/src/lib

can convert them as well if needed, I think this is just 5% of the codebase. I was really focused on plugins, so if plugins code look good...

zardoy

also let's remove duplicating code const registry = require('prismarine-registry')(version) and instead set it to server like serv.resitry = ...

zardoy · 2024-01-17T19:14:28Z

-        } finally {
-          process.stdout = tmp
-        }
+        // let tmp


need to revert i guess

what was the purpose of that? I remember it was broking output in some way

ok, got the issue, it worked before because the code was not in strict mode (which is not good)

zardoy · 2024-01-17T19:15:37Z

  function updateInventory () {
    playerData.inventory.forEach((item) => {
-      const itemName = item.id.value.slice(10) // skip game brand prefix
+      const itemValue: string | number = item.id.value


better inventory decode

zardoy · 2024-01-17T19:16:25Z

              if (result) {
-                serv.emit('banned', ctx.player ? ctx.player : { username: '[@]' }, username, reason)
-                if (ctx.player) ctx.player.chat(username + ' was banned')
+                serv.emit('banned', (ctx.player != null) ? ctx.player : { username: '[@]' }, username, reason)


its standard-ts changed al these checks

zardoy · 2024-01-17T20:43:40Z

I generated docs:
typedoc (without plugins) https://zardoy.github.io/space-squid/functions/createMCServer.html
real time generated docs from npm: https://paka.dev/npm/@zardoy/flying-squid

yes, need to clean up a few things like position: any or getSpawnPacket. But look at props like bannedPlayers that can reference other code. Imo all this looks useful.
But I'm just curious which generator do you think looks cleaner?

EDIT: there is a lot of noise on paka because it also included internal API, gonna update in a second

zardoy · 2024-01-18T15:26:05Z

  player.sendMap = () => {
-    return player.sendNearbyChunks(Math.min(3, settings['view-distance']))
-      .catch((err) => setTimeout(() => { throw err }), 0)
+    return player.sendNearbyChunks(settings['view-distance'])


the logic was hardcoded, I want to send more chunks

zardoy · 2024-01-18T15:27:55Z

+    parse (str) {
+      return str.match(/^(((.* )?~?-?\d* ~?-?\d* ~?-?\d*)|(.+ .+))$/) ? str.split(' ') : false
+    },
+    action (args, ctx) {


rewrite tp implementation (fixed it)

zardoy · 2024-01-18T15:29:12Z

+    const matches = allMatches.filter((match) => match.startsWith(existingContent))
    player._client.write('tab_complete', {
-      matches: matches.filter((match) => match.startsWith(existingContent))
+      matches: !registry.supportFeature('tabCompleteHasAToolTip')


fix crash on completions send in newer versions

zardoy · 2024-01-18T15:30:26Z

+    serv.playSound(sound, player.world, null, { ...opt, whitelist: player })
+  }
+
+  // player.on('placeBlock_cancel', async ({ reference }, cancel) => {


commented since events are not implemented in the code

zardoy · 2024-01-18T15:31:04Z

  }
  player._client.on('settings', ({ viewDistance }) => {
    player.view = viewDistance
+    player.sendRestMap()


bugfix, chunks with new view distance should be sended

zardoy · 2024-01-18T15:31:33Z

          arr.forEach(entity => {
            entity.takeDamage({ damage: 20 })
-            serv.info(`Killed ${colors.bold(entity.type === 'player' ? entity.username : entity.name)}`)
+            serv.info(`Killed ${colors.bold(entity.type === 'player' ? (entity as Player).username : entity.name ?? '<unknown>')}`)


entity can be without name in some cases

zardoy · 2024-01-18T15:32:14Z

  function attackEntity (entityId) {
    const attackedEntity = serv.entities[entityId]
-    if (!attackedEntity || (attackedEntity.gameMode !== 0 && attackedEntity.type === 'player')) return
+    const attackedPlayer = attackedEntity.type === 'player' ? attackedEntity as Player : undefined


yes a bit more verbose, but doesn't it look cleaner?

zardoy · 2024-01-18T15:38:30Z

    op: true,
-    parse (args, ctx) {
-      args = args.split(' ')
+    parse (_args, ctx) {


the ts pattern.

zardoy · 2024-01-18T15:41:58Z

-  player.on('disconnected', () => serv.info(player.username + ' disconnected'))
-  player.on('chat', ({ message }) => serv.info('<' + player.username + '>' + ' ' + message))
+  player.on('disconnected', (reason) => serv.info(player.username + ' disconnected. Reason: ' + reason))
+  // player.on('chat', ({ message }) => serv.info('<' + player.username + '>' + ' ' + message))


chat event not implemented yet

zardoy · 2024-01-18T15:43:22Z

      // esbuild custom plugin
-      const files = require(/* webpackIgnore: true */ 'esbuild-import-glob(path:.,skipFiles:index.js,external.js)')
-      module.exports.builtinPlugins = Object.values(files)
+      const files = require(/* webpackIgnore: true */ 'esbuild-import-glob(path:.,skipFiles:index.js,external.js,index.ts,external.ts)')


since we now have build step I think it would be better to remove these workarounds and instead create index.js with all plugins included. So we don't need to handle different platforms

zardoy · 2024-01-18T15:45:07Z

@@ -180,7 +181,7 @@ module.exports.player = function (player, serv, { version }) {
        player.changeBlock(data.position, 0, 0)
        const aboveBlock = await player.world.getBlock(data.position.offset(0, 1, 0))
        if (aboveBlock.material === 'plant') {
-          await player.setBlock(data.position.offset(0, 1, 0), 0, 0)


setblock doesn't have 3rd arg, could lead to potential bug

zardoy · 2024-01-18T15:48:14Z

    })

-    if (type === 'near') sample.sort((a, b) => a.position.distanceTo(opt.pos) > b.position.distanceTo(opt.pos))
+    if (type === 'near') sample.sort((a, b) => a.position.distanceTo(opt.pos) - b.position.distanceTo(opt.pos))


I believe this sorting was incorrect 😄 at least that one was caught by ts

zardoy · 2024-01-18T15:48:15Z

    })

    sample = sample.filter(s => {
+      changeType<Player & Entity & { team, scores }>(s) // TODO implement team & scores


an easy way to change any type without writing a bunch of unnecessary TS stuff

zardoy · 2024-01-18T15:49:58Z

-      const data = parseInt(params[5] || 0, 10)
-      const stateId = registry.supportFeature('theFlattening') ? (blocks[id].minStateId + data) : (id << 4 | data)
+      const data = parseInt(params[5] || '0', 10)
+      const stateId = postFlatenning


setblock was setting incorrect default state, fixed

zardoy · 2024-01-18T15:50:52Z

    serv.players
      .filter(p => p.world === player.world && player !== p)
-      .forEach(p => p.sendBlock(position, blockType, blockData))
+      .forEach(p => p.sendBlock(position, blockType/* , blockData */)) // todo


was handled incorrectly anyway

zardoy · 2024-01-18T15:52:50Z

-const UserError = require('./user_error')
+import UserError from './user_error'
+
+type Ctx<P extends boolean> = P extends true ? {


i tried to not add a lot of generics, but this one here is just super useful for other plugins

zardoy

found a few unrelated to js->ts migration, they are not big, but i can remove them if they are blocking this pr

zardoy · 2024-01-18T15:57:32Z

yes, I agree, typedoc is not the best generator here and its default look is really bad, but at least everything is structured. Anyway, I'm happy with making it all work, now its up to you whether to use it or not. If you still don't want to have anything like this, we can simply removing old docs (but I wouldn't be happy to keeping them up to date)

zardoy · 2024-01-18T16:01:48Z

the current build time is not ok, if everything else is ok, I will add esbuild-based pipeline for faster dev builds.

also I'm curious why !src/plugins/README.md is in .npmignore? whats the point of publishing it?

Pandapip1 · 2024-04-22T16:12:26Z

+1 to using typescript. Having even a small amount of type safety saves so much debugging time.

zardoy · 2024-04-22T17:54:00Z

saves so much debugging time.

You are right. If you write new modules or plugins you can catch bugs with the right types earlier.
But I'm doing it mostly for better DX and a way to pick definitions faster. This all just makes a huge difference for us and unfortunately, we simply can't work without types there. Also, unlike in mineflayer I can guarantee these types are correct since the source code reuses them. Btw can look at http://npmjs.com/package/@zardoy/flying-squid for a real demo

rom1504 · 2024-06-09T17:47:11Z

Tests still failing, my comments on libs/ not being converted is still valid.

Should we close this ?

zardoy · 2024-06-09T18:14:39Z

Tests still failing, my comments on libs/ not being converted is still valid.

I can fix tests if everything else is ok. Can you link the comments you are referring to?

Should we close this ?

Closing this one, means discarding a huge contribution, why don't you want to go in sync with my repo?

rom1504 · 2024-06-09T19:38:46Z

#651 (comment)

zardoy · 2024-06-10T00:39:13Z

#651 (comment)

Oh, ok, will do that, I will have time at the end of the week for this.

zardoy added 2 commits January 17, 2024 21:10

phase 1: simply rename to modules -> *.ts

f721622

(cherry picked from commit 3cbf1a4)

phase 2: add typings with script & a few manual fixes, fix a few bugs

90caa81

(cherry picked from commit 1acb152) with a lot of manual fixes

zardoy marked this pull request as draft January 17, 2024 16:18

zardoy commented Jan 17, 2024

View reviewed changes

zardoy added 14 commits January 17, 2024 23:24

add options types, fix some missing types

bb15d04

(cherry picked from commit 9748f4f)

fix: setblock now correctly sets initial block state when no override

ff4da02

remove almost all require's, make mcData typed (expose), make types ready for publishing! (cherry picked from commit e4be9d8)

make commands typed, fix many bugs! improve /tp

156c82a

add block renames (cherry picked from commit b9a2813)

fix tsc & use prepare for now

ef2393f

(cherry picked from commit f324dc4)

migrate docs to jsdoc with codemod, not documented are marked as inte…

76c5626

…rnal remove api.md for now fix gentypes (cherry picked from commit dd3253e)

fix: don't mutate generation options, fix storage provider is null fo…

8a38cdb

…r overworld (cherry picked from commit 8297df2)

fix tab_complete crash in post-flatenning

8eb54af

(cherry picked from commit 9d72f86)

add slash to aliases so its easier to discover them with search

7705c79

(cherry picked from commit 46edddc)

allow json resolve

0fc0670

(cherry picked from commit ba37e8d)

fix build

9f1fcdf

(cherry picked from commit b81e264)

add events, fix ts build

a6efc32

(cherry picked from commit 692f062)

improve many types, fix build!

0a2a798

try ts-standard instead

e8df1db

rename modules back to plugins

426969d

fix jsdoc newlines

4fd7926

zardoy commented Jan 17, 2024

View reviewed changes

zardoy mentioned this pull request Jan 17, 2024

Convert to typescript PrismarineJS/prismarine-auth#92

Draft

fix docs generator

9b5fe20

zardoy commented Jan 18, 2024

View reviewed changes

zardoy added 2 commits January 18, 2024 21:35

revert even more things...

51cceb4

finally test that it works

1fdc39f

zardoy marked this pull request as ready for review January 18, 2024 16:23

zardoy changed the title ~~TS 2~~ Full TypeScript rewrite. Best type defs Apr 18, 2024

rom1504 mentioned this pull request Mar 28, 2026

Ecosystem-wide: most promising open ideas across PrismarineJS PrismarineJS/mineflayer#3859

Open

Uh oh!

Conversation

zardoy commented Jan 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

socket-security Bot commented Jan 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zardoy commented Jan 17, 2024

Uh oh!

rom1504 commented Jan 17, 2024

Uh oh!

rom1504 commented Jan 17, 2024

Uh oh!

rom1504 commented Jan 17, 2024

Uh oh!

rom1504 commented Jan 17, 2024

Uh oh!

zardoy commented Jan 17, 2024

Uh oh!

zardoy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zardoy Jan 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zardoy commented Jan 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zardoy commented Jan 17, 2024 •

edited

Loading

socket-security Bot commented Jan 17, 2024 •

edited

Loading

zardoy Jan 18, 2024 •

edited

Loading

zardoy commented Jan 17, 2024 •

edited

Loading

zardoy commented Apr 22, 2024 •

edited

Loading