Page MenuHomeWildfire Games

Split CNetFileTransferer::HandleMessageReceive into one function per case
ClosedPublic

Authored by elexis on Jun 7 2018, 1:02 AM.

Details

Summary

Split this for good practice, making it a bit easier to read and maintain.

Test Plan

Notice the code didn't change. Could use an FSM model eventually.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Jun 7 2018, 1:02 AM
Vulcan added a subscriber: Vulcan.Jun 7 2018, 3:00 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/636/display/redirect

vladislavbelov added inline comments.
source/network/NetFileTransfer.cpp
29 ↗(On Diff #6738)

I think this place is good for switch.

43 ↗(On Diff #6738)

static_cast here and below.

106 ↗(On Diff #6738)

It'd be good to describe why exactly 0.5, and probably add a constant name.

elexis added inline comments.Jun 10 2018, 6:19 PM
source/network/NetFileTransfer.cpp
29 ↗(On Diff #6738)

So you disagree with the idea of this patch? I think we could move all the NetClient and NetServer functions into a single switch statement too.

106 ↗(On Diff #6738)

Or implementing the TODO

elexis added inline comments.Jun 10 2018, 6:22 PM
source/network/NetFileTransfer.cpp
29 ↗(On Diff #6738)

Sorry, didn't comprehend what you meant.

Preferably it should become an FSM but here we can use the switch indeed

elexis added inline comments.Sep 17 2019, 12:14 AM
source/network/NetFileTransfer.cpp
106 ↗(On Diff #6738)

I guess it's exactly 0.5 because that's 1/2 :p as in at most two LOG draws per second.
The comment of timer_Time() mentions the unit.
There should really be an event sent to the GUI.

elexis updated this revision to Diff 9815.Sep 17 2019, 12:16 AM

Switch, static_cast

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/727/display/redirect

The sprintf casts can be removed by using %lu for the u32 arguments and %zu for size_t. The latter has a precedent in the VertexBufferManager.cpp:

size_t vertexSize, size_t numVertices
LOGERROR("Failed to create VBOs (%zu*%zu)", vertexSize, numVertices);

http://www.cplusplus.com/reference/cstdio/printf/

source/network/NetFileTransfer.cpp
123 ↗(On Diff #9815)

We can reuse the iterator below to avoid the duplicate lookup. Also the [] operator has the side effect of insertion when most of the time it would be an error to insert this way.

source/network/NetFileTransfer.h
91 ↗(On Diff #9815)

private

This revision was not accepted when it landed; it landed in state Needs Review.Sep 17 2019, 4:20 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.