osu-framework icon indicating copy to clipboard operation
osu-framework copied to clipboard

`FlowContainer`'s layout position arguments should use a single number type

Open bdach opened this issue 4 years ago • 2 comments

It's a mix of int and float right now.

https://github.com/ppy/osu-framework/blob/758ec3418239d143c491ae92bfa7664ef6a650c1/osu.Framework/Graphics/Containers/FlowContainer.cs#L102-L126

Thinking it should be int everywhere (float is a strange data type to use for ordering, especially seeing that it has NaN which compares false to everything).

Aside from looking ugly this has been a cause of actual bugs in days past (see https://github.com/ppy/osu/issues/6399#issuecomment-538585390) which I only remembered now as it came up in discord discussions.

bdach avatar Aug 27 '21 20:08 bdach

The argument for float is that it is what we use for Drawable.Depth and generally a standard for graphics APIs. But I'm not against int as it is more definite and maybe more expected for something like a flowing list. Need more opinions for sure.

peppy avatar Aug 31 '21 14:08 peppy

I think the optimal solution would be to change Insert to accept float layout positions instead of int, and, if it becomes an issue, rename it to something else as to not conflict with List<T>.Insert(int, T).

frenzibyte avatar Aug 31 '21 19:08 frenzibyte