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

rename 'portable' to 'pickable' #1706

Merged
merged 3 commits into from
Jan 5, 2024
Merged

Conversation

kostmo
Copy link
Member

@kostmo kostmo commented Jan 4, 2024

Closes #1695.

Updating occurrences was pretty easy:

yq --inplace '(.[].properties[] | select(. == "portable")) |= "pickable"' data/entities.yaml

and

find data/scenarios -type f -name '*.yaml' -print0 | xargs --max-args 1 --null yq --inplace '(select(has("entities")) | .entities[] | select(has("properties")) | .properties[] | select(. == "portable")) |= "pickable"'

@kostmo kostmo changed the title Refactor/portable to pickable rename 'portable' to 'pickable' Jan 4, 2024
@byorgey
Copy link
Member

byorgey commented Jan 4, 2024

When we merge this we should also remember to regenerate affected wiki cheat sheets.

@kostmo kostmo marked this pull request as ready for review January 4, 2024 20:48
@kostmo kostmo requested a review from byorgey January 4, 2024 20:48
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

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

I see that we are doing #845 now too. 😄 I'm fine with that. Though I did notice a few things that need cleaning up. I didn't look at absolutely every single change so we should try searching for additional instances of the things I pointed out.

@kostmo
Copy link
Member Author

kostmo commented Jan 4, 2024

I see that we are doing #845 now too.

Will finish #1711 before merging this.

mergify bot pushed a commit that referenced this pull request Jan 5, 2024
Towards #845

This is to avoid the undesirable transformation by `yq` of whitespace-preserving string fields into quoted one-line strings, described in #1706 (comment).

Command:

    find data/scenarios -type f -name '*.yaml' -print0 | xargs -0 --max-args 1 sed -i -e 's/[[:blank:]]\+$//'
mergify bot pushed a commit that referenced this pull request Jan 5, 2024
YAML syntax in this file was actually fixed in #1672, which is not yet merged.
Cherry-pick that fix as well as enhancements to the scenario.
Towards #845.

Aside from fixing the syntax for #1706 (comment), this provides a good "before" example to showcase the fix in #1672.

    scripts/run-tests.sh --test-arguments '--pattern "1034-custom-attributes"'

and

    scripts/play.sh -i data/scenarios/Testing/1034-custom-attributes.yaml --autoplay --speed 1

![Screenshot from 2024-01-04 16-46-53](https://github.com/swarm-game/swarm/assets/261693/5e5a4435-1a36-4f59-bd6f-639c065baf2d)
@kostmo kostmo changed the base branch from main to refactor/normalize-entities-recipes January 5, 2024 18:29
@kostmo kostmo force-pushed the refactor/portable-to-pickable branch 2 times, most recently from e0b4f54 to 4f8516f Compare January 5, 2024 18:33
@kostmo kostmo force-pushed the refactor/portable-to-pickable branch from 4f8516f to a842cda Compare January 5, 2024 19:36
@kostmo kostmo force-pushed the refactor/normalize-entities-recipes branch from 5b1c3b1 to 7f2be5b Compare January 5, 2024 19:37
@kostmo kostmo force-pushed the refactor/portable-to-pickable branch from a842cda to 273cab1 Compare January 5, 2024 19:43
@kostmo kostmo changed the base branch from refactor/normalize-entities-recipes to main January 5, 2024 19:44
@kostmo kostmo force-pushed the refactor/portable-to-pickable branch from 273cab1 to c0f604a Compare January 5, 2024 19:46
@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Jan 5, 2024
@mergify mergify bot merged commit 47a8ffe into main Jan 5, 2024
10 checks passed
@mergify mergify bot deleted the refactor/portable-to-pickable branch January 5, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Portable" property is a misnomer
2 participants