Refactor: allow usage in browser #633

zardoy · 2023-09-03T08:20:02Z

Depends on PrismarineJS/node-minecraft-protocol#1254

fixes #491

Usage:

Both esbuild and webpack needs Node.js modules polyfilling.

esbuild: needs custom plugin like this one
webpack: needs module: { parser: { javascript: { commonjsMagicComments: true } } }

First of all, I'm really sorry for the delay, I wish I could do it early.

Anyway, right now I decided to squash all changes from here into one commit.

Will try to provide a brief overview of the changes I made here in comments.

Refactored plugin loading so its compatible with webpack (at least I tested the first version of it) and esbuild that I use currently.
I did initially a huge refactor by adding types, but I reverted them so its easier to review. Anyway, feel free to ask any questions

socket-security · 2023-09-03T08:24:10Z

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
exit-hook	2.2.1	None	`+0`	4.83 kB	sindresorhus

🚮 Removed packages: [email protected]

zardoy · 2023-09-03T08:27:29Z

package.json

@@ -21,7 +19,11 @@
    "test": "npm run mocha_test",
    "pretest": "npm run lint"
  },
-  "keywords": [],
+  "keywords": [


FYI just added some keywords (why not)

src/index.js

zardoy · 2023-09-03T08:36:22Z

src/index.js

@@ -38,23 +38,23 @@ function createMCServer (options) {

 class MCServer extends EventEmitter {
  constructor () {
+    plugins.initPlugins()


we now load plugins lazily

zardoy · 2023-09-03T08:37:47Z

src/lib/plugins/log.js

+if (isInNode) {
+  import(/* webpackIgnore: true */ 'exit-hook').then((hook) => {
+    hook.default(() => {
+      for (const serv of _servers) {


In the context of this refactor also fixed an important bug: worlds are not saved when server crashed or stopped by ctrl+c

I understand the intent and I agree the feature is nice. However that couple the lib and the app parts of flying-squid
What about instead define this as some ServerManager class and use it in app.js ?

What about exit-hook: this is totally okay to have it as part of the library and why not have it so "better behavior is enabled by default"?

Hm, as I understand there is also no way to disable readline, right?

"exit-hook: this is totally okay to have it as part of the library"

no that's exactly the part I don't like.

I think it's good if the library exposes a function that makes this possible, and then it's called by default in https://github.com/PrismarineJS/flying-squid/blob/master/app.js

having the library actually call it forces the user into a specific application setup

no that's exactly the part I don't like.

Alright, I don't mind making it an opt-in behavior, however, then I think it would make sense to make creating readline interface optional as well. What do you think?
Otherwise I might be missing use-cases of this library (sorry)

I think it's good if the library exposes a function that makes this possible

Will extract & doc it later today or tomorrow, not a problem

I think it's good if the library exposes a function that makes this possible, and then it's called by default

Maybe this should be done as option? like registerExitHook: true ?

src/lib/plugins/log.js

zardoy · 2023-09-03T08:43:29Z

src/lib/plugins/login.js

    try {
      const player = serv.initEntity('player', null, serv.overworld, new Vec3(0, 0, 0))
      player._client = client
+      player._client.socket ??= { remoteAddress: '' }


What about instead checking if it's defined where it's used ?

I removed mocking remoteAddress since nothing will crash if it is undefined, but maybe it should be documented in some way I don't really know (so other plugins can know it can be undefined)

where did you remove it ? I don't understand

Yeah, sorry for now making it clear I meant this 18fe14a . However I removed mocking : 6dff61f is it better?
I'm open to any option here.

zardoy · 2023-09-03T08:44:20Z

src/lib/plugins/login.js


 module.exports.server = function (serv, options) {
  serv._server.on('connection', client =>
    client.on('error', error => serv.emit('clientError', client, error)))

  serv._server.on('login', async (client) => {
-    if (client.socket.listeners('end').length === 0) return // TODO: should be fixed properly in nmp instead
+    if (client.socket?.listeners('end').length === 0) return // TODO: should be fixed properly in nmp instead
+    if (!serv.worldsReady) throw new Error('World is still preparing.')


As I remember this is another bugfix: when a player connects when world is not ready yet it crashes

what about returning instead of throwing if this is not an error ?

Yes of course it makes sense I fixed it here 923a829

zardoy · 2023-09-03T08:44:34Z

src/lib/plugins/login.js

-        theItem = mcData.blocksByName[itemName]
-      }
+      const itemName = item.id.value.slice(10) // skip game brand prefix
+      const theItem = mcData.itemsByName[itemName] || mcData.blocksByName[itemName]


just refactored thats it

zardoy · 2023-09-03T08:45:15Z

src/lib/plugins/login.js

    player.emit('spawned')

    await player.waitPlayerLogin()
    player.sendRestMap()
    sendChunkWhenMove()
+
+    player.save()


by doing this a new entry in playerdata/ appears immediately after a new player joins

src/lib/plugins/world.js

rom1504 · 2023-09-03T10:56:49Z

src/lib/plugins/log.js


 module.exports.server = function (serv, settings) {
+  _servers.push(serv)


What is _servers ?

I thought it was possible to create multiple servers. However, now I understand that I could just inline exit-hook handler here.

rom1504 · 2023-09-03T11:01:29Z

Overall looks good, but left some comments

rom1504 · 2023-12-31T14:18:31Z

overall lgtm, merged

rom1504 · 2023-12-31T14:18:46Z

/makerelease

zardoy added 3 commits September 3, 2023 10:55

Refactor: allow usage in browser

07bc819

move method to player

e88e6ad

fix bad require

a00cc94

zardoy added 4 commits September 3, 2023 11:24

fix standard

c5a4938

add basic tsconfig

5bd78f3

remove unused dep: didn't notice because of packages resorted in d

d4adf18

[skip ci] rm comment

a41b9b4

zardoy commented Sep 3, 2023

View reviewed changes

rom1504 reviewed Sep 3, 2023

View reviewed changes

src/lib/plugins/world.js Outdated Show resolved Hide resolved

rom1504 reviewed Sep 3, 2023

View reviewed changes

src/lib/plugins/world.js Outdated Show resolved Hide resolved

rom1504 reviewed Sep 3, 2023

View reviewed changes

zardoy added 5 commits September 3, 2023 14:35

remove mocking remoteAddress

18fe14a

make event: playerChangeRenderDistance, up docs

24c8c5e

introduce pluginsReady prop + event

923a829

revert auto-fix changes

0d06893

doc player.save

dfb8c33

zardoy and others added 6 commits September 5, 2023 11:57

dont init player on its disconnect

9f5c73c

seems got the problem

edb2193

[skip ci] remove server.options

4412cff

don't mock socket

6dff61f

add serv.formatMessage

3f875da

Merge branch 'master' into refactor-browser

5aa62fe

rom1504 merged commit 768b69b into PrismarineJS:master Dec 31, 2023
11 checks passed

rom1504bot mentioned this pull request Dec 31, 2023

Release 1.7.0 #642

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: allow usage in browser #633

Refactor: allow usage in browser #633

zardoy commented Sep 3, 2023 •

edited

Loading

socket-security bot commented Sep 3, 2023 •

edited

Loading

zardoy Sep 3, 2023

zardoy Sep 3, 2023

zardoy Sep 3, 2023

rom1504 Sep 3, 2023

zardoy Sep 3, 2023

zardoy Sep 3, 2023

rom1504 Sep 4, 2023

zardoy Sep 5, 2023 •

edited

Loading

zardoy Sep 8, 2023

zardoy Sep 3, 2023

rom1504 Sep 3, 2023

zardoy Sep 3, 2023

rom1504 Sep 4, 2023

zardoy Sep 5, 2023

zardoy Sep 3, 2023

rom1504 Sep 4, 2023

zardoy Sep 5, 2023

zardoy Sep 3, 2023

zardoy Sep 3, 2023

rom1504 Sep 3, 2023

zardoy Sep 3, 2023

rom1504 commented Sep 3, 2023

rom1504 commented Dec 31, 2023

rom1504 commented Dec 31, 2023


		module.exports.server = function (serv, settings) {
		_servers.push(serv)

Refactor: allow usage in browser #633

Refactor: allow usage in browser #633

Conversation

zardoy commented Sep 3, 2023 • edited Loading

socket-security bot commented Sep 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zardoy Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rom1504 commented Sep 3, 2023

rom1504 commented Dec 31, 2023

rom1504 commented Dec 31, 2023

zardoy commented Sep 3, 2023 •

edited

Loading

socket-security bot commented Sep 3, 2023 •

edited

Loading

zardoy Sep 5, 2023 •

edited

Loading