Switched to generic endian swap

[gnawme]

Thanks a lot for making a contribution to SFML! 🙂

Before you create the pull request, we ask you to check the follow boxes. (For small changes not everything needs to ticked, but the more the better!)

• Has this change been discussed on the forum or in an issue before?
• Does the code follow the SFML Code Style Guide?
• Have you provided some example/test code for your changes?
• If you have additional steps which need to be performed list them as tasks!

---

Description

Switched to use generic (templated) endian swap method that I've used successfully in the past.

This PR is related to the issue #2290

Tasks

• Tested on Linux
• Tested on Windows
• Tested on macOS
• Tested on iOS
• Tested on Android

How to test this PR?

Extended the Packet stream tests to include float and double

[codecov]

Codecov Report

Merging #2380 (508603c) into master (97c00d4) will increase coverage by 4.46%.
The diff coverage is 91.17%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2380      +/-   ##
==========================================
+ Coverage   27.15%   31.61%   +4.46%     
==========================================
  Files         228      198      -30     
  Lines       19637    14318    -5319     
  Branches     4710     2672    -2038     
==========================================
- Hits         5332     4527     -805     
+ Misses      13826     9644    -4182     
+ Partials      479      147     -332     

Impacted Files	Coverage Δ
src/SFML/Network/Packet.cpp	23.69% <0.00%> (-9.24%)	⬇️
src/SFML/Network/TcpListener.cpp	0.00% <ø> (ø)
include/SFML/Network/Packet.hpp	94.44% <96.87%> (+19.44%)	⬆️

... and 75 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97c00d4...508603c. Read the comment docs.

[ChrisThrasher]

If I understand correct, this PR would make sure bytes always get swapped around but what about the case where the machine's endianness matches network endianness? In that case we don't need to swap bytes around but this would seemingly still swap them.

[ChrisThrasher]

ChrisThrasher@26ae62e

This inspired me to write tests that make sure we're using the correct network ordering. Let me know if you can run your code with these tests and see if anything fails.

[JonnyPtn]

I'm not convinced SFML should be doing any reordering - in the vast majority of cases it's going to be little endian machines on both sides, using SFML sockets. In which cases the conversion is just redundant extra work for both sides

I think the only use case this benefits would be people communicating with other services they have no control over, in which case they can still do the conversion themselves?

At best this should be optional, and off by default

[gnawme]

ChrisThrasher@26ae62e

This inspired me to write tests that make sure we're using the correct network ordering. Let me know if you can run your code with these tests and see if anything fails.

Where can I access these tests?

[ChrisThrasher]

master...ChrisThrasher:SFML:test_network_order

[ChrisThrasher]

A lot of these functions are collapsing down to nearly identical code. I wonder if this new pattern means we can replace a lot of these function overloads with a template and cut out hundreds of lines of code.

[gnawme]

A lot of these functions are collapsing down to nearly identical code. I wonder if this new pattern means we can replace a lot of these function overloads with a template and cut out hundreds of lines of code.

I was thinking about that. Is there a consistent pattern that can be applied to character strings? Those are also not converted to network byte order.

[ChrisThrasher]

Is there a consistent pattern that can be applied to character strings? Those are also not converted to network byte order.

Each character is one byte which means the endianness doesn't really matter. If each character is >1 byte then I'd expect each character to get its bytes rearranged but not to reverse the string itself.

[ChrisThrasher]

I'm going to pause on the PR feedback until some other maintainers can weigh in on whether they like reimplementing sf::Packet. I believe that we can use a template with a closed set of specializations to simplify how many of these << and >> overloads are implemented much like how we explicitly specialize the sf::Vector2<T> template for floating point types to avoid having to put quite so much of that code in the header.

[gnawme]

believe that we can use a template with a closed set of specializations

I imagine that would be the subject of a separate PR

[ChrisThrasher]

I imagine that would be the subject of a separate PR

I see it as part of justifying this PR. It's easier to get this approved and merged this if it also means getting to delete huge swaths of code.

[gnawme]

I see it as part of justifying this PR.

Any sense of when and how the maintainers want to move forward?

[ChrisThrasher]

Any sense of when and how the maintainers want to move forward?

Sorry I'm not sure.