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

Support lexical ordering for macro types and rects #234

Closed
wants to merge 1 commit into from

Conversation

@rsaarelm
Copy link

rsaarelm commented Nov 18, 2017

This allows using Euclid types in BTree collections.


This change is Reviewable

This allows using Euclid types in BTree collections.
@rsaarelm
Copy link
Author

rsaarelm commented Nov 18, 2017

This is mathematically iffy since lexical ordering has no geometrical motivation. It is how the native arrays and tuples work in Rust though. Feel free to reject if the mathematical interpretation part is a sticking point.

Copy link
Collaborator

nical left a comment

Looks good to me, out of curiosity what use cases do you have in mind for ordering rects?

@rsaarelm
Copy link
Author

rsaarelm commented Nov 19, 2017

I want to store them in a set and I want the set iteration order to be deterministic. I'm working on a procedural map generator for a game, and I need it to be deterministic for a given Rng state. Hash containers introduce unspecified iteration order which breaks this, BTree containers are deterministic.

My current use case involves a set of Point2D values, so I don't have a need for Rects yet. I'm already using HashMap keys that contain Rects elsewhere in my code, so it's quite possible I'll come up with a map generator algorithm in the future that wants to store a bunch of rects in a set with a deterministic iteration order.

@kvark
Copy link
Member

kvark commented Nov 20, 2017

Hmm, I don't see PartialOrd implemented for either cgmath or nalgebra vectors, fwiw.

@rsaarelm
Copy link
Author

rsaarelm commented Nov 21, 2017

After making the PR I found out that it's reasonably painless to make a newtype wrapper with the lexical Ord implementation for the vector types, so there's maybe less need for this feature upstream than I originally thought.

@nical
Copy link
Collaborator

nical commented Nov 21, 2017

I am personally mostly indifferent as whether to implement arbitrary ordering for geometric types (erring on the side of "hm that's a bit weird"), @kvark has some reserves (understandably) but agrees to do this if there is a strong need.

If you don't need it too much, perhaps we should not merge this for now, but agree to revisit in the future if it turns out that there are compelling use cases that aren't painlessly worked around by newtypes or something else.

@rsaarelm
Copy link
Author

rsaarelm commented Nov 22, 2017

Yeah, maybe it's best to leave this unmerged for now and see if anyone else asks for this.

@nical
Copy link
Collaborator

nical commented Jan 30, 2018

Let's close this for now.

@nical nical closed this Jan 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.