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 Mac compatibility issues for Chess game. #61

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

Conversation

josecodes
Copy link

@josecodes josecodes commented Feb 19, 2025

Fixes a compatibility issue on mac caused by a chess piece filename case sensitivity issue. Since mac os treats q.png and Q.png (for example) as the same, it was only allowing one file to exist at a time. This caused issues obviously, including git weirdness.

So the filenames were changed to "bq.png" and "wQ.png" (for example) and the requisite code changes for the chess piece images to be rendered correctly were made.

To test the image filename change via PrettyRenderWrapper render of Chess, I needed to fix an issue with a null content variable problem for the game state that was causing the browser page to hang. That issue that this commit fixes is likely not just happening on macs; I suspect it is a problem on any OS when readme instructions are missing when rendering via PrettyRenderWrapper.

@DylanASHillier
Copy link
Contributor

Ah support this issue, was also seeing this

@DylanASHillier
Copy link
Contributor

TBH I question this rendering code being in the package here at all... its strange to send the javascript code to the frontend to help in rendering. At the very least the javascript should probably be saved as a .ts file and then loaded in by the get_custom_js function. but this is very out of scope

In any case maybe use more descriptive names for the files since they have to be remapped anyway? e.g. white_knight instead of wn. Also change the alt text to match the descriptive name?

@josecodes
Copy link
Author

josecodes commented Feb 21, 2025

@DylanASHillier I updated filenames and alt text to be more descriptive. Makes sense!

As far as javascript in a python file, yeah I thought it was strange too. I created a separate GitHub Issue for this to be dealt with in the future; it doesn't seem urgent but a good idea. It may also have a bigger scope; this pattern may exist and need to be updated in other renderers.

@josecodes
Copy link
Author

FYI @bobbycxy, this is the new PR for the mac compatibility issue that I closed a PR for earlier, that you were assigned to. Not sure if this PR got lost in the shuffle and should have an assignee.

@bobbycxy bobbycxy self-assigned this Feb 24, 2025
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.

3 participants