Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Miscellaneous TRR fixes #704
Miscellaneous TRR fixes #704
Changes from 17 commits
7422354
6c4dc4f
51414cb
481b4f8
33bdc71
b65a08f
9148a7b
02eb952
cb19c59
2b0dfb0
af9f593
5b70ecf
65b6cb5
386e045
091eeed
500b2d0
3cdee22
f6fe6df
7f7e633
fab296b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems safer and easier to understand than the reverse iteration through a list that is being modified.
Alternatively could the check be added above? Aanother
entityClean
step - then it just wouldn't be added in the first place.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I've gone with the approach suggested. I think it's safest to do this at the end so that the full list is iterated first in case there are two types with the same dependency, like
MaskedGoon2
andMaskedGoon3
, who both depend onMaskedGoon1
. Then we're not doing the same dependency check twice.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be a fairly simple looking Linq one-liner if
Room
knew what number room it was - but perhaps something for the future.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that would be good to implement. I don't think we shuffle rooms around anywhere so it's probably not something that would need management, but can have a think about it for the future. I've tidied it up for now anyway with some Linq so it's less loopy, and there is a test to cover it as well.