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

Piece adjustments #16805

Merged
merged 21 commits into from
Jan 21, 2025
Merged

Piece adjustments #16805

merged 21 commits into from
Jan 21, 2025

Conversation

johndoknjas
Copy link
Contributor

Positioning adjustments for some of the pieces in pirouetti, chess7, and staunty. I made a python script to do the adjustments automatically, and have included it in the PR. If not wanted though I'll remove it.

Copy link
Member

@fitztrev fitztrev left a comment

Choose a reason for hiding this comment

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

Nice, if it stays (I think it can) I'd recommend moving the python script to bin/gen and make it executable like licon.py.

@fitztrev
Copy link
Member

It looks like there's some functionality overlap with piece-css.ts (run via pnpm run piece-css). There's a github action workflow that runs it to make sure the css is in sync with the SVGs (.github/workflows/piece-sets.yml).

Maybe that's ok as long as they don't conflict. Just making you aware.

@johndoknjas
Copy link
Contributor Author

@fitztrev Oh true, hadn't noticed that. Does the ts script just check for them being in sync, or does it go further and overwrite the css using the svgs?

@fitztrev
Copy link
Member

Does the ts script just check for them being in sync, or does it go further and overwrite the css using the svgs?

It overwrites the css too. Then when the action workflow runs it, it checks the output to make sure there's no difference to what's committed (it verifies they're in sync that way).

@ornicar ornicar merged commit bf4857b into lichess-org:master Jan 21, 2025
4 checks 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.

3 participants