feat: basic support for text_display entity #230
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Phoenix616
force-pushed
the
pr/basic-text-display-support
branch
3 times, most recently
from
4a91471 to
fc92a5c
Compare
Phoenix616
force-pushed
the
pr/basic-text-display-support
branch
from
fc92a5c to
1e023f2
Compare
|
Oh, wow, probably the best pr so far! will check within 12h |
| @@ -454,6 +464,7 @@ export class Entities extends EventEmitter { | |||
| // --- | |||
| // not player | |||
| const displayText = entity.metadata?.[3] && this.parseEntityLabel(entity.metadata[2]) | |||
| || entity.metadata?.[23] && this.parseEntityLabel(entity.metadata[23]) // text displays | |||
There was a problem hiding this comment.
Is the key (index number) consistent across the versions that support it? We can check it entities data of minecraft-data module
There was a problem hiding this comment.
Also, it is definitely not right here! The 3 index is custom_name_visible property for entities. However 23 for text_display is text and for block_display is block_state property (not text), so it will result in error spamming in the console. So we should do a check here, however I might think of a better solution. Definitely code needs to be refactored (I'll gladly help you here). At least we should use property names, not number :d
There was a problem hiding this comment.
sadly dont have permissions to push
There was a problem hiding this comment.
sadly dont have permissions to push
Sorry, forgot that GitHub treats org forks differently from personal ones :S Will make PRs from a personal one in the future.
Also, it is definitely not right here! The
3index iscustom_name_visibleproperty for entities. However 23 for text_display is text and for block_display isblock_stateproperty (not text), so it will result in error spamming in the console. So we should do a check here, however I might think of a better solution. Definitely code needs to be refactored (I'll gladly help you here). At least we should use property names, not number :d
You are right, I didn't consider that. I kinda thought the indexes are entity-specific but it makes sense that they aren't. Feel free to leave PRs open in the future for me to fix such stuff directly in them. Will open another PR which improves the situation here (and maybe even adds more features of the text displays already, will have to look at how difficult to do that is with the current name tag system)
zardoy
changed the title
|
Ok I don't know when it will be done and I can't push changes here anyway so I will just merge it as is |
This adds basic rendering for the text of text displays. This doesn't include any other options like transforms, rendering modes or formatting of the text but makes them at least kinda usable on servers that use them to display important information. This is done by simply using the name tag system already built-in and expanding it to have multi-line support.
How it looks:
