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

Make State pub(crate). #42

Merged
merged 2 commits into from
Jan 22, 2025
Merged

Make State pub(crate). #42

merged 2 commits into from
Jan 22, 2025

Conversation

andriyDev
Copy link
Collaborator

Users shouldn't be messing with the internal state of the behavior tree. That could result in the behavior tree producing all sorts of strange behavior.

@andriyDev
Copy link
Collaborator Author

Fixed rebase conflicts. Unfortunately it looks like there's a new lint or something because this PR is failing due to State suffixes on all the State variants. I'd like to address this in a separate PR (or more likely as part of #41).

Therefore I think this PR is fine to move forward with.

@andriyDev
Copy link
Collaborator Author

Ahhh actually, it looks like the lint by default only applies to enums that aren't public (the logic being that it would be annoying if introducing a lint resulted in a breaking change).

I will fix this now then.

Copy link
Owner

@Sollimann Sollimann left a comment

Choose a reason for hiding this comment

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

Looks good, makes sense to remove State! You need to rebase and bump minor version now since changing the API (I don't want to move this crate to major version 1 yet )

ref semantic versioning https://semver.org/

Let me know when comments have been addressed and I'll approve it :)

@andriyDev
Copy link
Collaborator Author

I rebased, but I think to handle the version bump we should just merge #38. Otherwise these will both just have the same commit so one will get dropped anyway (I'm gonna have to do a rebase again once either of them is merged anyway).

@Sollimann Sollimann self-requested a review January 15, 2025 02:47
Users shouldn't be messing with the internal state of the behavior tree. That could result in the behavior tree producing all sorts of strange behavior.
@andriyDev
Copy link
Collaborator Author

Sorry for the delay! I didn't notice the approvals. Rebased onto #38.

@andriyDev andriyDev merged commit d3a1f84 into Sollimann:main Jan 22, 2025
2 checks passed
@andriyDev andriyDev deleted the private-state branch January 22, 2025 06:24
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