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: support for empty file names #436

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

feat: support for empty file names #436

wants to merge 5 commits into from

Conversation

edayot
Copy link
Contributor

@edayot edayot commented Apr 4, 2024

The function with an empty name .mcfunction is currently not supported by beet atm.

This PR fixes this issue by modifying the code where the extensions of a file are calculated (in beet/library/utils.py).

Q/A

  • Where does the problem come from ? : In the python pathlib, the first suffix is skip if the filename start with a dot

  • Why is it a problem ? : Because minecraft loads these functions as usual, and other datapackers rely on this behavior.

@edayot edayot marked this pull request as ready for review April 4, 2024 15:00
@edayot edayot marked this pull request as draft April 4, 2024 15:02
@edayot edayot marked this pull request as ready for review April 4, 2024 15:34
@vberlier
Copy link
Contributor

vberlier commented Apr 11, 2024

Arf, I apologize for leaving your issue unanswered for so long! I remember that back when I first saw the issue, I thought that allowing empty names could cause problems in other places because beet plugins often just assume that the name can't be empty. I wanted to look into it more in depth but eventually I just forgot and never got back to you.

I still don't have a clear answer though. I've always felt like Mojang doesn't even know that empty names are allowed. If it was never intended, they could very well make it an error the moment they realize and a bunch of packs would break. It just seems like a bug that's bound to be fixed sooner or later so I generally avoid relying on it.

But it's true that as a build tool, beet should strictly conform to the game's behavior. I'll have to look through a couple plugins to see if additional changes are required to take into account empty names.

@edayot
Copy link
Contributor Author

edayot commented Apr 11, 2024

Yeah it always seemed very hacky to me to use empty filenames, just writing function namespace: feels weird, but people are using it.
Do i have to push the files in test/snapshots/** to make the test pass ?

@vberlier
Copy link
Contributor

Yeah you have to commit the snapshots

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