Bug #1710
closed
disaster frequency value > 255 cause network problem
Added by Dean Brown 15 days ago.
Updated 9 days ago.
Description
In packets.def the disaster frequency is UINT8, needs to be UINT32. The ruleset value could in theory be as high as 1000000 if you wanted the disaster to always happen. Currently values > 255 get truncated.
If game.ruleset has -
[disaster_5]
...
frequency=50000
get message -
"Trying to put 50000 into 8 bits; it will result 80 at receiving side."
Pretty rare, so I suggest targeting 3.3.0
Files
Related issues
1 (1 open — 0 closed)
Dean Brown wrote:
In packets.def the disaster frequency is UINT8, needs to be UINT32. The ruleset value could in theory be as high as 1000000 if you wanted the disaster to always happen. Currently values > 255 get truncated.
The max should be 1000, I think, as it gets multiplied by the server setting that has max of 1000 (1000*1000=1000000). The server setting values that user may set should be meaningful, or the ruleset should lock the setting (to 1000).
Marko Lindqvist wrote in #note-2:
Dean Brown wrote:
In packets.def the disaster frequency is UINT8, needs to be UINT32. The ruleset value could in theory be as high as 1000000 if you wanted the disaster to always happen. Currently values > 255 get truncated.
The max should be 1000, I think
So correct type would be uint16. It's not that using the two extra bytes would be any way significant, but in the name of self-documenting code uint16 would show the intendet usage better.
- Target version set to 3.3.0
- Target version changed from 3.3.0 to 3.3.0-npf
- Blocks Tasks #1712: S3_3 network protocol freeze (npf) added
My example of frequency=50000 was a mistake on my part, I was wrong about what my server setting was. With server setting "disasters", 1000 I can change the usual disaster's frequency to 1 without changing their actual result chance of .001, and then frequency=255 for my added disaster gets me to .255, much closer to what I want. So 1000 as a max value seems right, and UINT16 is OK. Updated the patch.
There is no bounds checking in the ruleset load code, except implicit "read an int" must limit value to 2**31.
- File deleted (
1710.patch)
- Status changed from New to In Review
- Assignee set to Marko Lindqvist
- Status changed from In Review to Closed
Also available in: Atom
PDF