Skip to content

Conversation

@Argmaster
Copy link
Collaborator

Extracted from #1207

@Argmaster Argmaster marked this pull request as ready for review March 23, 2025 12:45
Copy link
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

This PR is doing too many things at once, making it difficult to review.
Please do the indentation changes in a later PR and only extract the helper function here.

Github is bad at displaying changes if you change the indentation and move code around. So please do them separately. (Or at least make separate commits, so I can look at them separately, but I've told you this already)

@Argmaster
Copy link
Collaborator Author

Here. Since the code was duplicated for readAllObjFilesInAddonsHashmap I replaced it there too.

@Argmaster Argmaster changed the title Extract createAssetStringID from readAllZonFilesInAddons Extract createAssetStringID Mar 23, 2025
@Argmaster Argmaster closed this Mar 23, 2025
@Argmaster
Copy link
Collaborator Author

Ok. I am refusing to figure out every possible edge case that is a result of this design decision.

@IntegratedQuantum
Copy link
Member

All I said is that you should print an error if there is a dot before the extension (or only remove the extension, as it was before). Everything else is outside the scope of this PR anyways.

@Argmaster Argmaster reopened this Mar 24, 2025
@Argmaster
Copy link
Collaborator Author

Argmaster commented Mar 24, 2025

Here, for ".zig.zon" there is special case that removes it, for everything else there is generalized removal of last suffix, hopefully I didn't mess up the indexes. idk, they don't like me and I don't like them.

Copy link
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

That's better, thank you.
(Sadly didn't save any lines, but we'll get there with the next read*Files function)

@IntegratedQuantum IntegratedQuantum merged commit d9a4e7c into PixelGuys:master Mar 24, 2025
1 check passed
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.

2 participants