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.

Sign up for GitHub

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

Jump to bottom

Added Origin related nametag prefix #521

Open
wants to merge 6 commits into
base: 1.20
Choose a base branch
from
Open

Added Origin related nametag prefix #521

wants to merge 6 commits into from

Conversation

RaymondBlaze
Copy link
Contributor

@RaymondBlaze RaymondBlaze commented Apr 14, 2022 •
edited
Loading

Added features in response to #491:

  • Players now have their display name prefixed with their Origins (default style: [Origin Name])
  • Prefixes are sorted in OriginLayer's order and is hidden along with OriginLayer
  • Added field nametag_prefix of type Text in Origin so that datapack/addon developers can customize the prefix.
  • Added client server config to toggle the render of the prefix

This PR should be tested in multi player game, but I haven't got a chance to do so ;)

@JadeJuno
Copy link
Contributor

Question, because I don't know where to find this in the code: What is the config set to by default?

Also, I feel like the config should also be server-side?-

@apace100
Copy link
Owner

I have to agree with Golder that this should probably be a server-side setting.

@RaymondBlaze
Copy link
Contributor Author

  1. Moved to server side :)
  2. Added a blacklist config so that players can chose to show important origin layer only (eg. the extra layer in Libra may have its name too long and pointless to show)
  3. Also noticed some inappropriate usages of PlayerEntity#getDisplayName in project, should have redirect to use PlayerEntity#getName which directly referencing the actual player name instead. It's not this PR's mixin's problem that these usages have to be redirected though, as PlayerEntity#getDisplayName is already potentially prefixed by player's team. In that case, usages in Apoli and Calio should also be redirected.

@RaymondBlaze
Copy link
Contributor Author

What is the config set to by default?

Currently, the default value is true, showing the prefix, as I am making a active respond to #491 :)
Technically, this PR should only affect SMP, as display name is not so often seen in single player (?). In that case, defaulting to true is more lovely from my POV.

@MerchantPug
Copy link
Contributor

MerchantPug commented May 19, 2022 •
edited
Loading

I'd much prefer this to default to false.

Setting it to true by default would just make people question why it suddenly appeared on their servers.

Also consider Human players who may not want to interact with the origins mod.

@eggohito
Merge branch '1.20' into feat/origin_nametag_prefix
ccb2e89 
# Conflicts:
#	src/main/java/io/github/apace100/origins/Origins.java
#	src/main/java/io/github/apace100/origins/command/OriginCommand.java
#	src/main/java/io/github/apace100/origins/component/PlayerOriginComponent.java
#	src/main/java/io/github/apace100/origins/networking/ModPacketsC2S.java
#	src/main/java/io/github/apace100/origins/origin/Origin.java
@eggohito eggohito changed the base branch from 1.18 to 1.20 July 30, 2023 17:46
@eggohito
Changed a few method calls for getting the player's name
682a05d 
To be consistent throughout the project
@eggohito
Copy link
Collaborator

I've now changed the base branch of this PR to 1.20.x and updated it accordingly. I've also changed the default value of the config to false as per Pug's suggestion. I'll test this in a bit and see if there are any issues :]

eggohito added 2 commits July 31, 2023 01:59
@eggohito
Update NametagPrefixMixin (oops)
ab20fc0
@eggohito
Fixed NametagPrefixMixin
a35d996 
* Name is now actually appended to the prefix (oops)
* A whitespace is now appended to the prefix (before the name is appended)
eggohito
eggohito approved these changes Jul 30, 2023
View reviewed changes
Copy link
Collaborator

@eggohito eggohito left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Aside from the errors I made (and fixed) on merging 1.20.x into this PR, everything looks good 👍 It seems to work just fine in a singleplayer and multiplayer environment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eggohito eggohito eggohito approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@RaymondBlaze @JadeJuno @apace100 @MerchantPug @eggohito