Blizzard / s2client-api

StarCraft II Client - C++ library supported on Windows, Linux and Mac designed for building scripted bots and research using the SC2API.
MIT License
1.66k stars 281 forks source link

Point2D <--> Point2DI #160

Closed davechurchill closed 7 years ago

davechurchill commented 7 years ago

It would be convenient if Point2D had a constructor which took in a Point2DI and vice versa.

Also, if all the PointXX classes had a < operator, so we could use them for keys in std::map

AnthonyBrunasso commented 7 years ago

Sure I can do this.

In regards to the operators:

Less than operator is what? Every component in the left hand side vector is less than the one in the right hand side vector?

davechurchill commented 7 years ago

It can't be every component, because that would cause incomplete orderings.

You can just sort first by X, if X is tied sort by Y, and if Y is also tied sort by Z. Think of it as treating each element of the vector as a digit of a number and then you're just doing less than on the 2 numbers.

jswigart commented 7 years ago

That's a very specific function to use an operator for. seems like a situation you should provide the sort function to the map yourself.

If I saw an operator on a vec3 I would expect it to mathematically make sense, and probably do something like return a.x < b.x && a.y < b.y && a.z < b.z, so that it could be useful for a mathematical purpose.

davechurchill commented 7 years ago

"Mathematical" less than on a vector doesn't make any intuitive sense, and should never be used for that purpose. Even the example you gave I cannot come up with a use case for. What does it mean for all 3 elements of a vector to be less than another vector? The bigger issue than this is that the ordering is not strict, meaning that you could end up with a different sorted list of vectors based on their initial starting positions, which is akin to undefined behavior would be really bad.

The 'less than' operator here that I've requested is simply a strict ordering over points that is used for purposes of comparison for mapping, sorting, etc, which are very useful to have for many reasons. To say it's very specific probably means you've never tried to make a std::set of points, or make a map where the key is a point. Both of which are quite common, and have many uses in a bot.

Since the mathematical concept of 'vector less than' doesn't make any real sense or have any real applications, I doubt anyone would be using it for any other purpose.

jswigart commented 7 years ago

A PointInAABB function would use such a mathematical implementation to check if a point were less than all components of the mins and greater than all components of the maxs. I'm not saying that implementation would fulfill the requirements of a sorting. It obviously wouldn't. I think it's a fine idea to provide a static function within the class for a sorting function, but the coding "purist" rebels a bit at the idea of co-opting a mathematical operator for it(even though i realize that is what the implementation of map operates on) with an implementation that is contrary to what one might expect from an operator. It's not my library though, so it's not my call, and really it's not a big deal either. In hindsight it's too minor of a nitpick.

jswigart commented 7 years ago

I think you are more correct that the expectations of such an operator in the context of a vector3 is too ill defined for it to have a consistent implementation, so might as well use it to allow the class to just work in a sorted container. In the examples that would use such logic, the programmer would be making a risky decision to rely on that implementation of the operator, and would be better off writing that code in their PointInAABB function directly.

davechurchill commented 7 years ago

I agree with the mathematical purist in you - but we are dealing with C++ here, so we have to live by its conventions :)

N00byEdge commented 7 years ago

I agree with the sorting that @davechurchill proposed, but I find that just as easily the operator< can be overriden by your bot, or you can pass a sorting lambda. std::pair and std::tuple compare by first comparing the first value, then the second one and so on (lexiographically).

It should probably even be asked if these classes should inherit from their native C++ counterparts (std::tuple etc) to automatically get the behavior expected from those classes.

AnthonyBrunasso commented 7 years ago

I think there's enough ambiguity in what a less than operator on a point would do that it's probably not worth adding to the API. If it is simply to use a sorted container like std::map a functor can be added as a templated argument upon instantiation.