Skip to content

Treesanity#5541

Merged
Malkierian merged 13 commits intoHarbourMasters:developfrom
serprex:treesanity
Oct 2, 2025
Merged

Treesanity#5541
Malkierian merged 13 commits intoHarbourMasters:developfrom
serprex:treesanity

Conversation

@serprex
Copy link
Contributor

@serprex serprex commented May 26, 2025

Updated #5003 with latest dev (by hand, because merge conflict hell)
Extended to all trees in HF
Cleaned up code

Build Artifacts

@serprex serprex marked this pull request as ready for review May 27, 2025 04:53
@serprex serprex force-pushed the treesanity branch 3 times, most recently from c3b0069 to 85e2087 Compare May 27, 2025 16:03
@TheLynk
Copy link
Contributor

TheLynk commented May 28, 2025

Where are the tree 14 15 16?
image
image

@serprex
Copy link
Contributor Author

serprex commented May 29, 2025

I hit 14/15/16 when I run around bonking trees, can you try changing your coordinate to the coordinates in ShuffleTrees.cpp?

@TheLynk
Copy link
Contributor

TheLynk commented May 29, 2025

image
image
image
Nothing XD

@Malkierian
Copy link
Contributor

Malkierian commented May 29, 2025

It looks like there are more trees as child. For instance, this is as adult from the grotto tree:
image
And this is from virtually the same location, but as child. You can see the trees that were there as adult, but there are several that just weren't there as adult:
image

@TheLynk
Copy link
Contributor

TheLynk commented May 29, 2025

I finally understood they are exclusive to children's age and not adult so logic problem

@serprex
Copy link
Contributor Author

serprex commented May 29, 2025

Thanks for testing this, logic updated

@TheLynk
Copy link
Contributor

TheLynk commented May 29, 2025

I confirm that there are the 3 shafts miss that they appeared following your update

I have an idea to offer you but I don't know if it's frightening but you think it is possible that the tree that they are child and adult you could separate them to increase the number of checks?
For example, a child in child age does not give the same check in adult age

@serprex
Copy link
Contributor Author

serprex commented May 29, 2025

I would prefer not to do that, as it contradicts other shuffles & lookup based on X/Z coordinates would need to be adjusted

@TheLynk
Copy link
Contributor

TheLynk commented May 29, 2025

OK, no problem

Copy link
Contributor

@Pepper0ni Pepper0ni left a comment

Choose a reason for hiding this comment

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

A few fixes and comments. It would also be nice to have always hints for the NL trees when they are on, but I wouldn't block over that.

@Malkierian
Copy link
Contributor

Were you aware of #5003 when making this?

@TheLynk
Copy link
Contributor

TheLynk commented Jun 12, 2025

Yes he knows how to look at the first message he put at the beginning

@Pepper0ni
Copy link
Contributor

As stated in the OP, this was a deliberate re-implementation because the old branch was stale and the dev unresponsive.

@Malkierian
Copy link
Contributor

I assume the by hand part was just re-applying the changes to a later codebase, and that's why their commits aren't in the history? Just want to know for future reference so I can do a co-author of the original PR's dev when this actually gets merged, if applicable.

@serprex
Copy link
Contributor Author

serprex commented Jun 13, 2025

@Malkierian correct, lots of copy pasting

@serprex serprex marked this pull request as ready for review June 21, 2025 14:15
@serprex serprex force-pushed the treesanity branch 2 times, most recently from fc796fe to 169e6df Compare June 21, 2025 16:10
Copy link
Contributor

@Pepe20129 Pepe20129 left a comment

Choose a reason for hiding this comment

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

Did a run and found no issues with functionality, some small suggestions.

@Malkierian Malkierian merged commit 545cc39 into HarbourMasters:develop Oct 2, 2025
6 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.

5 participants