JannisX11 / blockbench

Blockbench - A low poly 3D model editor
https://www.blockbench.net
GNU General Public License v3.0
3.07k stars 272 forks source link

Fix textures `relative_path` being incorrectly relative to a non-dir #2150

Closed AlexTMjugador closed 2 months ago

AlexTMjugador commented 6 months ago

Blockbench may store the relative paths to model textures in its native .bbmodel project format when used as a standalone application. However, this path is relative to the project file instead of the project file directory, which generates invalid relative paths, because such paths are only meant to be relative to directories and not files, as shown in the following figure:

Node REPL showing invalid relative paths The first parameter passed to path.relative in the image above represents a possible value of Project.save_path, while the second is an absolute texture path. The expected output would be texture.png or ./texture.png, not ../texture.png.

Fix this problem by calling dirname on the project save path that's used to relativize texture paths. Even though this quirk is not documented anywhere and thus I would expect it to be unnoticed by most people, it is nevertheless a technically breaking change, so please let me know if you wish to bump the format version because of this. The relative paths generated after this change have been tested to work well across platforms.

AlexTMjugador commented 6 months ago

I have rebased the PR on top of next because it looks like the intended branch for these less stable changes, judging from the history of PRs that have been merged into the repository.

I also realized that I probably didn't elaborate enough on the backwards compatibility implications of this change, so I want to mention that this may also affect how old project files are loaded in Blockbench versions with these changes. In practice, though, I'd expect that to be a fairly minor issue, as Blockbench can usually fall back to data URLs and absolute paths if relative paths are invalid, but again, please let me know if you'd like to see these edge cases handled differently :smile:

JannisX11 commented 2 months ago

Good catch! This was fixed in 4.10 beta 1 as a versioned change, along with a format version increase.