ClanGenOfficial / clangen

Warrior Cats fan game
https://clangen.io
Other
229 stars 398 forks source link

Bugfix: change how the directory is found #2521

Open j-gynn opened 6 days ago

j-gynn commented 6 days ago

About The Pull Request

This is a very minor fix to address a bug when attempting to run some development monitoring tools - __file__ is undefined.

I think we may need to review how the error handling works, as if it gets a valid directory (which I believe it always will with this specific code), it can continue. I don't know what a robust check for this might look like though.

Why This Is Good For ClanGen

It allows me (and others) to use some more beefy testing tools that were otherwise thrown off by the use of the __file__ parameter.

Proof of Testing

https://github.com/ClanGenOfficial/clangen/assets/48025294/bdc9d3ca-55ec-4cc6-8fb4-fef8c2b3f6d5 (hopefully this is legible enough - after the cut I am forcing an error to show that it throws an exception).

Changelog/Credits

Bugfix to enable use of dev tools

larkgz commented 5 days ago

Some things I would like confirmed:

I don’t think it should affect these things, but I’d like it confirmed first because the paths can get kind of weird with those situations.

j-gynn commented 5 days ago

I will send out a request for some Mac beta testers, but this is a well-documented approach. os.path.realpath does have some differences due to how different OSes handle symbolic links, but as we're only operating on one individual's system and not using hardcoded paths anywhere else, we shouldn't encounter issues. MacOS and Linux would actually allow a simpler version that Windows fails on!

j-gynn commented 5 days ago

I've just heard back from one of our Mac beta testers - they have been able to confirm that everything works on MacOS.

j-gynn commented 5 days ago

RE: the builds - I think that's tested in GH Actions? Edit: It is, but only when it's merged in. I think it's a bit late by then but... y'know. Not sure how else to test it - maybe someone can manually trigger that action on this branch?

j-gynn commented 5 days ago

Update: Linux users are reporting issues. Reverting to draft until I can figure out why. Dang Linux. /j

j-gynn commented 4 days ago

Following review, we have determined that the issue the Linux user was experiencing is not as a result of this branch, and as such I am putting this back to ready for review.