Conversation
| mutation.MutatedSpecies.Organelles.Approve(true); | ||
| } | ||
|
|
||
| mutation.MutatedSpecies.Organelles.Approve(false); |
There was a problem hiding this comment.
Should this be in an else clause? Because now if the if is true above then we first all Approve and then Approve false.
src/general/HexLayout.cs
Outdated
| return existingHexes.Contains(item); | ||
| } | ||
|
|
||
| public void Approve(bool isApproved = true) |
There was a problem hiding this comment.
Why is this method named Approve? Doesn't calling Approve(false) mean that this is rejected? I think for clarity it would be nicer to have two methods: Approve and Reject without any parameters each.
|
|
||
| IEnumerator IEnumerable.GetEnumerator() | ||
| { | ||
| return GetEnumerator(); |
There was a problem hiding this comment.
This causes boxing due to the way the enumerator is implemented. Hopefully this is not called much?
There was a problem hiding this comment.
No, this shouldn't be called at all.
There was a problem hiding this comment.
Maybe a GD.PrintErr warning about the performance implications then? (if in the future this somehow ends up being called)
There was a problem hiding this comment.
This also happens in the current HexLayout implementation, by the way, also in the standard GetEnumerator (which is used in 66 places, Rider tells me)
There was a problem hiding this comment.
But there's a comment with a TODO about this very issue.
There was a problem hiding this comment.
I think this case is different, because this is a struct so any allocated enumerator will have a copy of the struct data and not just a reference to the data. So I kind of think that this is a slightly more problematic situation than a simple lightweight enumerator allocation. But I admit I don't fully know how structs having an enumerator function in them behave.
xfractalino
force-pushed
the
hex-layout-optimization
branch
from
2bcd594 to
48f85e9
Compare
2 participants
Brief Description of What This PR Does
Avoiding deep-cloning for the organelle layout and hex layouts in general by using temporary pooled memory for diffs only.
Progress Checklist
Note: before starting this checklist the PR should be marked as non-draft.
break existing features:
https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
(this is important as to not waste the time of Thrive team
members reviewing this PR)
styleguide.
Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.