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

Limit ItemLink Name to 16 Characters #4318

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Exempt-Medic
Copy link
Member

@Exempt-Medic Exempt-Medic commented Dec 1, 2024

What is this fixing or adding?

Aliases and player names are both limited to 16 characters, however, ItemLink names are not. This truncates them to sixteen characters as well.

Also changes the error message to be more specific about the conditions for identical item link names and adds quotation marks around the name itself.

How was this tested?

Not all that much, generating an itemlink with the name "Just A Really Long Name" and both generating and uploading a world on a local webhost using the same itemlink group. Checked that the name everywhere had been shortened and connected to the multiworld to get some checks.

@github-actions github-actions bot added affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Dec 1, 2024
@Exempt-Medic Exempt-Medic added the is: enhancement Issues requesting new features or pull requests implementing new features. label Dec 1, 2024
@Jarno458
Copy link
Collaborator

Jarno458 commented Dec 2, 2024

We never truncated item and location names tbh

@NewSoupVi
Copy link
Member

NewSoupVi commented Dec 2, 2024

This will result in weird exceptions when the name overlaps, right? I wonder if the Exception could be a bit more verbose here

Something like this (extremely loose and unoptimized code)

if len(name) > 16:
    name = name[:16]
    if name in ...:
        raise Exception("After being truncated, the name already exists. Item link names must differ in the first 16 characters")
    ...

Then again I'm often quite a lot more prone to writing a million user friendly exceptions than other devs, so maybe this is not necessary

@Exempt-Medic
Copy link
Member Author

Exempt-Medic commented Dec 2, 2024

We never truncated item and location names tbh

An alias and player name can be shown when getting .player from something. They were limited to avoid breaking various games/clients, but itemlinks breaks them anyway

@Exempt-Medic
Copy link
Member Author

Exempt-Medic commented Dec 2, 2024

This will result in weird exceptions when the name overlaps, right? I wonder if the Exception could be a bit more verbose here

Could do that, I just borrowed the already used barebones style

new_name = new_name.strip()[:16].strip()

if new_name == "Archipelago":

raise Exception(f"You cannot name yourself \"{new_name}\"")

@Exempt-Medic
Copy link
Member Author

Exempt-Medic commented Dec 2, 2024

if len(name) > 16:
    name = name[:16]
    if name in ...:
        raise Exception("After being truncated, the name already exists. Item link names must differ in the first 16 characters")
    ...

So this isn't technically true because of names like " I Had Spaces At The Start". But I doubt the specificity of the exact requirements is worth putting into the error message, so "the first 16 characters" sounds great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants