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

disable construct intent for buggy zones #1403

Merged
merged 2 commits into from
May 11, 2024
Merged

disable construct intent for buggy zones #1403

merged 2 commits into from
May 11, 2024

Conversation

chrisfarms
Copy link
Contributor

there's a contract bug preventing the CONSTRUCT_BUILDING_MOBILE_UNIT action for working as intended when the zone id is 40 or more.

as a temporary measure we are disabling the build intent for the affected zones so that it reduces the potential confusion of either the resulting "out of bounds" error or the silent failing due to these errors only being debug visisble.

there's a contract bug preventing the CONSTRUCT_BUILDING_MOBILE_UNIT
action for working as intended when the zone id is 40 or more.

as a temporary measure we are disabling the build intent for the
affected zones so that it reduces the potential confusion of either the
resulting "out of bounds" error or the silent failing due to these
errors only being debug visisble.
@5p0rt5BEArD 5p0rt5BEArD self-requested a review May 11, 2024 16:27
// fails for zone ids above 40. As a temporary measure to limit confusion we
// will disable the construct button on these zones while we consider options
// see: https://github.com/playmint/ds/issues/1402
const isBuggyZone = (id: number) => id >= 40;
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be >= 41?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

was erring on the safe side of the off-by one ness

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm going to test it

Copy link
Contributor

@5p0rt5BEArD 5p0rt5BEArD May 11, 2024

Choose a reason for hiding this comment

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

>= 41 is correct

I've confirmed the following two statements locally

  • The bug is present on zone 41 but not on zone 40
  • The button is not displayed for zone 41 but is displayed for zone 40 with const isBuggyZone = (id: number) => id >= 41;

Copy link
Contributor

@5p0rt5BEArD 5p0rt5BEArD left a comment

Choose a reason for hiding this comment

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

👍

@chrisfarms chrisfarms merged commit 3a85229 into main May 11, 2024
3 checks passed
@chrisfarms chrisfarms deleted the disable-build-btn branch May 11, 2024 17:17
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.

2 participants