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

[core] Lay out casts and null checks more around gardening #4282

Closed
wants to merge 4 commits into from

Conversation

zach2good
Copy link
Contributor

I affirm:

  • I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • I have read and understood the Contributing Guide and the Code of Conduct.
  • I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

Steps to test these changes

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
sdispater Sébastien Eustace
@zach2good zach2good marked this pull request as ready for review August 31, 2023 09:18
Comment on lines +105 to +109
bool ignoredDuringStage = !wasExamined && stageDuration > wiltTime;
bool ignoredSinceChange = !wasExamined && stageDuration + daysSinceStageChange > wiltTime;
bool shouldForceWilt = daysSinceStageChange > VANADAYS_TO_GUARANTEE_WILT + wiltTime;

if (ignoredDuringStage || ignoredSinceChange || shouldForceWilt)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I gave this logical split to downstream and they are currently testing it to see if one of these is redundant and possibly causing spurious wilting

@zach2good
Copy link
Contributor Author

I'm going to be pretty busy this week, if someone could quickly smoke test this against a day or two of gardening it would be much appreciated

@Xaver-DaRed
Copy link
Contributor

Xaver-DaRed commented Sep 5, 2023

I made a fresh build of your branch.

Maybe I'm doing something wrong, however, after placing the flowerpots, when attempting to feed it a seed to grow it, it will just ignore the imput. Won't consume the seed and the flowerpot will remain empty.

This happens with all 10 placed flowerpots.

@zach2good
Copy link
Contributor Author

I was hoping Horizon would have made some similar changes to this and had some feedback, but they haven't had time/bandwidth, and I don't know gardening well enough to weigh in on anything apart from how C++ works.

Closing, pending more reports/info

@zach2good zach2good closed this Aug 5, 2024
@zach2good zach2good deleted the gardening_safety branch August 5, 2024 09:13
KnowOne134 pushed a commit to KnowOne134/server that referenced this pull request Jan 28, 2025
* update promotion lance corporal

* fix capitalization of vars.Stage
zach2good pushed a commit that referenced this pull request Feb 15, 2025
* Lance Corp start files

* update promotion lance corporal (#4282)

---------

Co-authored-by: Duke <109076646+Dukilles@users.noreply.github.com>
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.

None yet

2 participants