Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prismarine: Fixup Entity class #1341

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Prismarine: Fixup Entity class #1341

wants to merge 1 commit into from

Conversation

enricoangelon
Copy link
Collaborator

Well, for the umpteenth time the usual mistake was happening: a lot of logic and not very functional, many useless methods, some even without an actual purpose and also disconnected from the logic of the game. By analyzing the Entity class I was able to find a uuid, which only players have, resulting in useless logic and not even set up by the game. The Position class must be deterministic, an entity MUST be in a world and in a position, it makes no sense to make the parameters optional, and it only creates damage and UB. A console that acts as a player is something that adds complexity in the code for nothing, sure, they have things in common (a name and that we both send messages), but they are two different concepts... it makes more sense to use a interface. Furthermore, the Entity class must NOT contain netcode in any way, in fact it has been removed. Surely many things will need to be reviewed now, but we will gain stability later on.

@enricoangelon
Copy link
Collaborator Author

It probably solves it #1331.

Copy link

codecov bot commented Apr 23, 2024

Bundle Report

Changes will increase total bundle size by 281.64kB ⬆️

Bundle name Size Change
@jsprismarine/errors-@jsprismarine/errors 9.8kB 468 bytes ⬆️
JSPrismarine-@jsprismarine/errors 9.8kB 468 bytes ⬆️
@jsprismarine/prismarine-@jsprismarine/prismarine 3.0MB 273.83kB ⬆️
@jsprismarine/color-parser-@jsprismarine/color-parser 6.58kB 0 bytes
JSPrismarine-@jsprismarine/prismarine 3.0MB 5.83kB ⬇️
JSPrismarine-@jsprismarine/color-parser 6.58kB 0 bytes
JSPrismarine-@jsprismarine/client 38.84kB 0 bytes
@jsprismarine/nbt-@jsprismarine/nbt 111.32kB 2.28kB ⬇️
JSPrismarine-@jsprismarine/nbt 111.32kB 0 bytes
@jsprismarine/brigadier-@jsprismarine/brigadier 296.93kB 16 bytes ⬆️
JSPrismarine-@jsprismarine/logger 19.71kB 0 bytes
@jsprismarine/client-@jsprismarine/client 38.84kB 0 bytes
JSPrismarine-@jsprismarine/server 8.55kB 184 bytes ⬇️
JSPrismarine-@jsprismarine/brigadier 296.93kB 11.92kB ⬆️
@jsprismarine/raknet-@jsprismarine/raknet 228.14kB 0 bytes
JSPrismarine-@jsprismarine/raknet 228.14kB 0 bytes
@jsprismarine/logger-@jsprismarine/logger 19.71kB 0 bytes
JSPrismarine-@jsprismarine/math 15.66kB 1.62kB ⬆️
JSPrismarine-@jsprismarine/minecraft 133.68kB 2.98kB ⬆️
@jsprismarine/math-@jsprismarine/math 15.66kB 1.62kB ⬆️
@jsprismarine/server-@jsprismarine/server 8.55kB 184 bytes ⬇️
JSPrismarine-@jsprismarine/protocol 137.09kB 2.81kB ⬇️

@@ -59,7 +57,7 @@ export class Vector2 {
* await entity.setZ(10);
* ```
*/
public setZ(z = 0): void {
public setZ(z: number): void {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

defaults here may lead to undefined behaviors.

@@ -191,6 +192,9 @@ export class Metadata extends MetadataWriter {
* @TODO: Add missing functions.
*/
protected setDefaults(): void {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As explained in the comments, this whole implementation is wrong.

import type ContainerEntry from '../../inventory/ContainerEntry';
import { AddItemActorPacket } from '../../network/Packets';
import { Position } from '../../world/Position';
import type { World } from '../../world/World';
import { Entity } from '../Entity';

export class Item extends Entity {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, an Item is an Entity, but its runtime ID isn't correlated to the Entity one, we will see about this one.

@filiphsps
Copy link
Member

uuid is used by the level manager, well partially at least (but that's mostly due to not being completed). Similar to how JE does it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants