Skip to content

Simplify Range<T> usage and implementation, switching to half-open (end is exclusive)#3965

Draft
YoshiRulz wants to merge 4 commits intomasterfrom
goodbye-generic-range
Draft

Simplify Range<T> usage and implementation, switching to half-open (end is exclusive)#3965
YoshiRulz wants to merge 4 commits intomasterfrom
goodbye-generic-range

Conversation

@YoshiRulz
Copy link
Member

@YoshiRulz YoshiRulz commented Jul 10, 2024

WIP, but the next part will be much harder, so if you want to review and merge don't wait for me do not merge without unit tests.

After merge, need to remember to update https://github.com/TASEmulators/BizHawk/wiki/Available-C%23-and-.NET-features

@Morilli
Copy link
Collaborator

Morilli commented Jul 10, 2024

What's the WIP part about this? Could this just be merged as is or is something missing?

@YoshiRulz
Copy link
Member Author

YoshiRulz commented Jul 10, 2024

It could be merged as-is. (It might be a good idea to have a unit test, at least before the refactor from closed to half-open, since that one changes some logic.) The part that's missing is to make Int32Interval half-open. It may also be possible to make them value types, in which case some or all uses of Int32Interval could be the BCL Range instead.

@SuuperW
Copy link
Contributor

SuuperW commented Dec 3, 2025

What is the advantage of this?
How is this simpler than the previous implementation, aside from removing unused code?
Why is Int32Interval closed while Int64Interval is half-open? EDIT: You said above that changing that is "missing". I say change it before merging (if it's going to ever be merged).

@YoshiRulz YoshiRulz mentioned this pull request Feb 22, 2026
2 tasks
@SuuperW
Copy link
Contributor

SuuperW commented Feb 22, 2026

If you want this merged prior to my PR, address my comments and get it merged.

@YoshiRulz YoshiRulz force-pushed the goodbye-generic-range branch from acf0ec5 to 464b94d Compare February 23, 2026 16:29
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.

3 participants