Proposal: Remove circular distance option, retire `rl_dist`.

[sparr]

Is your feature request related to a problem? Please describe.

The code around distance calculations is needlessly complex. The default changed from square_dist to trig_dist years ago, and I suspect almost everyone plays without turning circular distances off.

Solution you would like.

I want to retire this option and feature. Replace all calls to rl_dist with trig_dist, and keep using square_dist only in the places it's currently used explicitly.

Describe alternatives you have considered.

No response

Additional context

No response

[ZhilkinSerg]

It might make sense to also audit existing usage of square_dist (75 hits in 23 files) - it is possible we might want to use trig_dist almost everywhere

[kevingranade]

Please quantify "needlessly complex" and relate it to something with project impact like ongoing maintenance burden. Are we having ongoing issues with it breaking? Is it difficult to add new features because of the option?

I'm not completely against it's removal, but it also doesn't seem to accomplish enough to justify the overhead of removing it (risk of breaking things).

[sparr]

Please quantify "needlessly complex" and relate it to something with project impact like ongoing maintenance burden. [...] Is it difficult to add new features because of the option?

The inspirations for this post are my recent attempts to use the fast distance (deferred sqrt) functions in more places, and to resolve premature integer division. Both of these efforts were hampered by the dual nature of our dist functions, and both exposed me to many places in the code where previous coders had made small mistakes like thinking rl_dist returns a float when using circular distances.

[kevingranade]

The first thing is kind of vague, and the second doesn't seem to have anything to do with non-circular distances? It seems like more of an api consistency problem that exists independently of the underlying distance metric.

[sparr]

Yeah, the latter problem is more general. A more general proposal would be "Stop having two distance functions with two different return types"; just using octile everywhere (and returning a float!) would be better than using square dist some places (returning an int) and trig dist other places (returning an int or a float depending on how you get to it).

[kevingranade]

We will still want square dist, trig dist, and octile dist depending on the situation, it's not that simple 0_0

Though having them all return float is probably fine, add long as rounding is done correctly.

[sparr]

Oh, having them all to call explicitly when needed isn't a problem. The problem is that the one we call for general purposes (rl_dist) does two different things and everything downstream of it suffers for that.