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

feat: Increase maximum supported length of chest names #4076

Open
wants to merge 4 commits into
base: 1.4.4
Choose a base branch
from

Conversation

Metratrj
Copy link
Contributor

@Metratrj Metratrj commented Feb 24, 2024

What is the new feature?

Increase the MaxNameLength for chest names and implemented the usage of this variable instead of hardcoded (magic)number in the ChestUI and UIVirtualKeyboard.

Why should this be part of tModLoader?

Feature was requested by @direwolf420 in #3909.
To quote him:

  • The chest itself has a long name. This is in the realm of possibilities as one vanilla chest has a length of 19, 1 below the limit. If a mod adds a material with a corresponding chest it will say VeryLongMaterialName Chest but gets cut off.

  • Chat tags are used with modded items - Write [i:2] which displays dirt block, [i:ModName/ItemName] which displays ItemName. You can see immediately that the latter is almost impossible unless the mod and/or the item has a short internal name. This is useful for players that want to have a better visual feedback for item sorting purposes.

Are there alternative designs?

We could increase the hardcoded values, but it does not make much sense to me because the variable MaxNameLength already exist in the Chest class and take usage should make the thing easier to change for modders too.

Sample usage for the new feature

Rename chest with longer names ;)

ExampleMod updates

None

Closes #3909

Increased the MaxNameLength and start utilizing this variable
@Metratrj Metratrj marked this pull request as ready for review February 24, 2024 16:38
@Metratrj
Copy link
Contributor Author

@JavidPack, @Solxanich & @direwolf420 what do you think?

Copy link
Contributor

@direwolf420 direwolf420 left a comment

Choose a reason for hiding this comment

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

Review

Comment on lines +87 to +88
- public const int MaxNameLength = 20;
+ public const int MaxNameLength = 63;
Copy link
Contributor

Choose a reason for hiding this comment

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

Regarding it not being used as MaxNameLength in the source: Since tml uses decompiled vanilla code, const only appear in their literal value.
Regarding where it's used, look at what https://github.com/NotLe0n/BetterChests/blob/1.4.4/src/Edits/ChestNameEdits.cs edits, you are missing some places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added missing changes but utilize further this const

patches/tModLoader/Terraria/UI/ChestUI.cs.patch Outdated Show resolved Hide resolved
Metratrj and others added 3 commits February 27, 2024 10:08
revert changes in ChestUI
change doc for MessageID 33(SyncPlayerChest)
revert changes in ChestUI
change doc for MessageID 33(SyncPlayerChest)
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.

Increase maximum supported length of chest names
2 participants