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

fix: 🐛 Fix mod id validation #202

Merged
merged 1 commit into from
Mar 30, 2023

Conversation

Qubus0
Copy link
Collaborator

@Qubus0 Qubus0 commented Mar 30, 2023

really simple check for length 3
@ithinkandicode do we really need all of that type stuff at the beginning of the method? what was the intended use for that and will that be followed up upon?

@Qubus0 Qubus0 changed the base branch from main to development March 30, 2023 16:38
@KANAjetzt KANAjetzt added the validation Feature to make things safe and predictable label Mar 30, 2023
@KANAjetzt KANAjetzt added this to the v6.0.0 milestone Mar 30, 2023
@KANAjetzt
Copy link
Member

What type stuff exactly?
That one?

var intro_text = "A %s for the mod \"%s\" is invalid: " % [type, original_mod_id] if not type == "" else ""

@Qubus0
Copy link
Collaborator Author

Qubus0 commented Mar 30, 2023

yep that. it seems to only check if it is empty and it is only ever set as empty string.

@KANAjetzt
Copy link
Member

It's used in the error message to pass the field name.

not is_mod_id_array_valid(mod_id, dependencies, "dependency") or
not is_mod_id_array_valid(mod_id, incompatibilities, "incompatibility") or
not is_mod_id_array_valid(mod_id, optional_dependencies, "optional_dependency") or
not is_mod_id_array_valid(mod_id, load_before, "load_before")

@Qubus0
Copy link
Collaborator Author

Qubus0 commented Mar 30, 2023

oh I missed the usage in that. disregard that comment then

Copy link
Member

@KANAjetzt KANAjetzt left a comment

Choose a reason for hiding this comment

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

Approved ✅
Do we consider this a breaking change? 😬

@Qubus0
Copy link
Collaborator Author

Qubus0 commented Mar 30, 2023

yeah probably breaking. though name and namespace are validated with 3 characters already

@Qubus0 Qubus0 added this pull request to the merge queue Mar 30, 2023
Merged via the queue into GodotModding:development with commit 80fea72 Mar 30, 2023
@Qubus0 Qubus0 deleted the fix_mod_id_validation branch March 30, 2023 20:17
@KANAjetzt KANAjetzt added the breaking Breaking change label Apr 4, 2023
@ithinkandicode ithinkandicode changed the title Fix mod id validation fix: 🐛 Fix mod id validation Jun 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change validation Feature to make things safe and predictable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants