Pick all new changes #353

zardoy · 2023-09-19T14:26:34Z

Preview deploy: https://mcraft.fun or https://mcon.vercel.app - PCM.GG for short.
Hey! This PR adds all features from my fork without any exceptions. I rebased this version using my script so it doesn't introduce the viewer into the repo. Yes, I know there are a lot of commits, so I think I'll rebase it one or more times to squash some commits, but it already can give you a final vision of the project. Maybe I'll think of re-grouping some commits (and open some separate prs if you ask). There is a diff with my main branch. Again, the reason I'm opening this pr is to have some kind of initial feedback so I can better understand what things you need, what things you don't like, etc...
There are some notable changes:

migrated to esbuild. this is not because webpack was too slow, but because it was giving a lot of unpredictable behaviors. I spend countless hours debugging webpack dev & prod configs, esbuild config is just easier to maintain for me as there is no dev and prod modes. I'm not going back to chaos, see Make Webpack faster #210 (comment)
fully migrated to ts
react. didn't push these changes yet, but i have fully migrated from lit. the only reason for this is shadowdom. imo react easier to maintain at scale with its great ecosystem. I'm going to setup storybook for components soon.
on CI there are vercel deploy workflow and ci.yml with cypress testing (still working on some tests). But one integration test (singleplayer testing) already can cover a bunch of possible issues

fixes #10
fixes #33
fixes #47
fixes #117
fixes #210
fixes #253
fixes #241
fixes #274
fixes #287
fixes #304
fixes #302
fixes #66 (if you going to setup vercel project)
and many others not reported here...

general tasks:

remove my deps (12) (not possible currently)
remove vite and webpack configs & deps as they are not used anymore
Remove my links
Restore linting (eslint)
fix: no disconnect on: 1) wrong version specified 2) too long username 3) disconnect button click (but still sometimes no pong issues)

1. watchable for changes 2. can be used along with react

…l+w), but not when paused so can easily close if desired

add mobile button review upstream wip react refresh (thats why migrating to react) to migrate to tsx to impl advanced settings

…adding flying add settings like the one to disable in-game auto-update

inspect dragndroped nbt data! improve disconnect action (disconnect immediately)

TODO not pixel-perfect

socket-security · 2023-09-27T15:55:54Z

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Issue	Package	Version	Note	Source
Potential typo squat	eslint-config-zardoy	0.2.17	Did you mean: eslint-config-zaturdboy (16 thousand times more downloads)

Next steps

What is a potential typo squat?

Package name is similar to other popular packages and may not be the package you want.

Use care when consuming similarly named packages and ensure that you did not intend to consume a different package. Malicious packages often publish using similar names as existing popular packages.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

@SocketSecurity ignore eslint-config-zardoy@0.2.17

zardoy · 2023-09-27T15:59:48Z

I expect you to handle config/workflow files as you want. Feel free to guide me in the right direction. I'll stop adding new commits here as there is already extremely big.

.github/workflows/ci.yml

rom1504 · 2023-11-04T22:23:45Z

CI is failing. Can you fix ?

rom1504 · 2023-11-04T22:24:36Z

.eslintrc.json

@@ -0,0 +1,89 @@
+{
+    "extends": "zardoy",
+    "ignorePatterns": [


Can you remove this file ? We use standard

rom1504 · 2023-11-04T22:24:59Z

.github/workflows/ci.yml

+        uses: actions/checkout@master
+      - name: Install pnpm
+        run: npm i -g pnpm
+      - run: pnpm install


because of pnpm lock i guess

Ideally, it should not matter what pm you use, but I had to put lockfile into the repo so I can lock deps for fully reproducible builds (without sacrificing branch information stored in package.json). So, yes, you are forced to use pnpm for reproducible builds, but if you don't care you actually use any other pm.
i personally use pnpm because obviously, it is fast for external deps (gh, npm) (when I started working on all this my initial goal was to make ci as fast as possible). I hope it answers your question, but since it's not my project I don't mind any changes in workflow files (you decide)

Npm also supports package lock. Doesn't it ?

rom1504 · 2023-11-04T22:25:31Z

.vscode/launch.json

@@ -0,0 +1,61 @@
+{
+    "configurations": [


Please remove

why? how do you debug this project otherwise?

rom1504 · 2023-11-04T22:26:07Z

cypress/integration/index.spec.ts

@@ -0,0 +1,79 @@
+/// <reference types="cypress" />
+import type { AppOptions } from '../../src/optionsStorage'


Why is there typescript code here ?

rom1504 · 2023-11-04T22:26:28Z

cypress/minecraft-server.mjs

@@ -0,0 +1,26 @@
+//@ts-check
+import mcServer from 'flying-squid'


Does it really matter?

Why .mjs ?

Because I like import syntax highlight.
I generally more like esm because of top level await.
And yes I'm aware of esm restrictions (__surname and so on

We don't use .mjs anywhere else in PrismarineJS. Syntax highlighting works there though

Oh ok if you want me to remove it won't be a problem. You just asked why…
And no, syntax highlighting doesn't work in cjs files in the same way as it works in esm (I mean import keyword!)

rom1504 · 2023-11-04T22:27:18Z

package.json

  "dependencies": {
+    "@dimaka/interface": "0.0.3-alpha.0",
+    "@types/react": "^18.2.20",


Why react ?

I'm not the type of guy who just decided to make a full rewrite using my favorite frameworks. I removed lit in the latest version not only because React has a much greater ecosystem (such as good storybook support) & clean function components and I didn't see a single reason to use lit. I could adopt, really, but there was a thing that was blocking a lot of stuff. The main reason why I had to drop lit is the shadow dom (e.g. no way of using global CSS variables).
I've enjoyed the rewrite and am happy with the final result, but if you're not I'm sorry! I actually didn't see other solutions.

no way of using global CSS variables

this is one of the best feature of lit

this is one of the best feature of lit

How?

This is the same as saying that "no way to build scalable applications is also the best feature of lit". These kinds of restrictions don't make code more maintainable it just makes it harder to find solutions. Meanwhile react has the best ecosystem you can find if you're a frontend developer, it still is the fastest & easiest way to build robust UI at scale. Just take the current storybook integration: zero-config first-class react support. I tried to adopt lit, but it sucks at absolutely everything, no good state management, no hmr, plugins and so on... Also, my team could easily work with react + ts and even build new components without any problems so I don't see objective points here, for most other devs it doesn't really make the difference what framework you use...

global css makes it impossible to build reusable components, it's not much more complicated than that

that's why react doesn't really have reusable components but instead it has sets of components you need to all use together

beyond that, regarding what framework to use here, I am not yet convinced we need anything big as react / state management / etc

I guess the only way forward here is for me to go and pick stuff that makes sense from your fork, and eventually we converge to something reasonable

But React is not that big and at some points even easier to learn. You just need to understand how Function components work and that's. And what is more reasonable from your point? This is an absolutely fantastic & simple UI framework (any frontend guy would say you the same), I don't understand what's the issue with that (sorry)

Hi, I'm the frontend guy and React is pretty bad, actually. There's much simpler ones like Svelte, and more performant ones like Solid. This project probably uses Lit for a good reason, like reducing bundle size, and we need to respect that (unless majority agrees to migrate to something else).

rom1504 · 2023-11-04T22:28:14Z

src/controls.ts

+      interactPlace: [null, 'Left Trigger'],
+      chat: [['KeyT', 'Enter'], null],
+      command: ['Slash', null],
+    },


Why is this in typescript?

If there is an enforcement to not use it, it won't be a problem for me to not add the blue bar to your repo. I added it for my personal needs (to speed up development & gain confidence and for some other contribs as well). I should not care about your infra decisions, but it would be great if you could participate and say what you want to be picked. I will also try to get another dev that could probably adopt the infra changes.
That's also the reason why I threw away the idea of rebasing & splitting it into smaller (a bit) prs. I just need to understand what is allowed here and what is not. Again, some kind of task list would be an ideal solution. You can hop to my repo if you want some kind of build preview. Also, please feel free to ask anything, I can explain anything you want.

I could barely read more than 500 commits (I believe the commit history could be much cleaner) and I noticed a huge number of various important fixes so I would not call this just a small bag of improvements. That’s why I really I wish it could be available some day (really want to try it out), so @zardoy why did you have to do this unnecessary infra changes, why you couldn’t skip this part!? Also, doesn’t make ts harder to contribute as you need to learn & write types?

rom1504 · 2023-11-04T22:30:44Z

Too bad I didn't have time to review before but yeah in the current state we can't merge.

This is a mixed bag of good improvements and unnecessary infra changes.
Idk what to do with it.

If you were to propose each large change independently it would be possible to discuss each change and eventually reach something we can merge.

rom1504 · 2023-12-17T16:54:32Z

tried https://mcraft.fun/ today

I got to admit the single player mode is pretty cool

let's see how we can get this merged

rom1504 · 2023-12-17T23:03:00Z

can you think of some ways to first include functional changes here with no refactoring ?

or do technical PRs with no functional changes ?

Saiv46 · 2024-01-08T17:32:00Z

I was about to start fixing #210 by migrating to esbuild, but holy shit this is a huge PR.

This will probably not gonna to be merged anytime soon in one piece. Can we try to split it into smaller ones?

Saiv46 · 2024-01-08T17:40:55Z

I'll leave it here: zardoy#1 (fork's roadmap, includes fixes)

zardoy · 2024-01-08T21:25:00Z

@Saiv46 I understand that and I went tooo far with this one. Even this pr is just extremely outdated (tho I didn't do esbuild pipeline changes much recently), and I wish I had more time & energy to split to more prs here. I was too interested in implementing anything and couldn't focus on small prs much at that time (not great I know).

Can we try to split it into smaller ones?

I literally wanted to hire someone to help me with this one, if you could collaborate it would be really really good. As was said in zardoy#47 some things probably would be better to pick from the main branch (if you are interested), let me know where we can start

I'll leave it here: zardoy#1 (fork's roadmap, includes fixes)

hm, what do you mean?

zardoy · 2024-01-08T21:28:26Z

but holy shit this is a huge PR

btw as I remember I needed one hour to just read the code changes, but esbuild pipeline changes are not that big

rom1504 · 2024-06-09T17:09:15Z

To clarify here, this cannot be merged as is.

You have 2 ways forward

Introduce changes one at a time to PrismarineJS
Or simply rename your repo to ZaydoyGUI or whatever name you like

zardoy added 30 commits September 19, 2023 16:27

[pnpm] enable shell-emulator for new contributors on windows

8d3d5ca

simple fov change on sprint (so sprint can be indicated)

2a0533c

refactor modal stacks so they are:

aeeb012

1. watchable for changes 2. can be used along with react

feat: display leave confirmation when playing (prevent accidental ctr…

edf1def

…l+w), but not when paused so can easily close if desired

inventories are almost here wip:

bb26208

add mobile button review upstream wip react refresh (thats why migrating to react) to migrate to tsx to impl advanced settings

rm log

7b08d17

Merge remote-tracking branch 'origin/main' into inventory

bc474cc

add ci.yml

42a11dd

make render distance updatable, refactor controls spend some time on …

714bf3d

…adding flying add settings like the one to disable in-game auto-update

make new ci.yml run on pulls

e519e69

replace legacy lib, comment

2e3f30a

flying check for spectator

50b5c76

play -> connect to server

37a3a7b

fix opening chat

860ed43

[experimental] allow to override num of workers

8a563ae

feat: preserve chat history in sessionstorage

0285401

support player saving data 🚀

8e385c6

inspect dragndroped nbt data! improve disconnect action (disconnect immediately)

support world saving 🚀

3c8200a

refactor: move browserfs to its own file

d58867b

temporary fix for block model in inventory gui

b405289

add support for icons in button

b1b03f0

TODO not pixel-perfect

feat(big!): implement folder save loading with write access

7eb2d54

add ci.yml

2ea2f53

Merge branch 'main' into inventory

6deb405

build should be fixed (disable inventory for now)

359930d

Remove linked dep

6f6f089

wip testing cypress

1d895a3

install false

e5f1810

Offline saving + world folder read / write! (#8)

ff52875

Merge branch 'main' into cypress

947f76a

zardoy added 4 commits September 27, 2023 18:46

fix most annoying ts-js issue here I guess

5bdf626

manual build fixes

f60e363

remove my links

295253c

run preview on every pr commit

0633d8d

zardoy force-pushed the main-light branch from 78e7fdb to 0633d8d Compare September 27, 2023 15:52

preview: simplify checkout

9bb29a3

zardoy commented Sep 28, 2023

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

rom1504 reviewed Nov 4, 2023

View reviewed changes

.eslintrc.json

@@ -0,0 +1,89 @@

{

"extends": "zardoy",

"ignorePatterns": [

Copy link

Member

rom1504 Nov 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove this file ? We use standard

rom1504 reviewed Nov 4, 2023

View reviewed changes

rom1504 closed this Jun 9, 2024

zardoy mentioned this pull request Jun 10, 2024

Contribute changes back to original repo zardoy/minecraft-web-client#47

Open

		@@ -0,0 +1,79 @@
		/// <reference types="cypress" />
		import type { AppOptions } from '../../src/optionsStorage'

		@@ -0,0 +1,26 @@
		//@ts-check
		import mcServer from 'flying-squid'

Pick all new changes #353

Pick all new changes #353

Conversation

zardoy commented Sep 19, 2023 • edited Loading

socket-security bot commented Sep 27, 2023

Next steps

zardoy commented Sep 27, 2023 • edited Loading

rom1504 commented Nov 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zardoy Nov 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

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 Nov 4, 2023

rom1504 commented Dec 17, 2023

rom1504 commented Dec 17, 2023

Saiv46 commented Jan 8, 2024

Saiv46 commented Jan 8, 2024 • edited Loading

zardoy commented Jan 8, 2024

zardoy commented Jan 8, 2024

rom1504 commented Jun 9, 2024 • edited Loading

zardoy commented Sep 19, 2023 •

edited

Loading

zardoy commented Sep 27, 2023 •

edited

Loading

zardoy Nov 5, 2023 •

edited

Loading

Saiv46 commented Jan 8, 2024 •

edited

Loading

rom1504 commented Jun 9, 2024 •

edited

Loading