CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
10.58k stars 4.16k forks source link

Debug Menu Wind Speed labels are incorrect #63624

Closed KKDavion closed 1 year ago

KKDavion commented 1 year ago

Describe the bug

Under Debug -> (m)ap -> wind (s)peed, all listed speeds are labeled as being m/s. However, using a wind turbine and the Weather Reader CBM as reference, each of these settings seem to force the speed in mph.

Attach save file

Power Test-trimmed.tar.gz

Steps to reproduce

  1. Debug install Weather Reader CBM
  2. Debug place Wind Turbine Appliance
  3. Force wind in debug menu to 0 m/s. Note the Turbine reading +0 W & the CBM reading 0 m/s
  4. Force wind in debug menu to 30 m/s. Not the Turbine reading +30 W & the CBM reading 13.4 m/s

Expected behavior

For the debug menu's labels to be in MPH, or for the speed forced to match.

Screenshots

No response

Versions and configuration

Additional context

I'd fix this myself but it looks like it involves C++, which I'm still a little too scared to touch

Also, mostly unrelated, but PR #50395 states that turbines should generate power at 1:1 by m/s. This is untrue as they seem to generate power 1:1 by mph, which I've seen corroborated online & on the discord. I'm doing a small look into power generation right now so that number might change either way.

Hirmuolio commented 1 year ago

Maybe adding units::speed to remove unit ambiguity from code would help (miles/h, km/h, meter/s, tiles/turn). But since distances and speeds are a bit handwaved in some areas that may cause new problems.

KKDavion commented 1 year ago

I did a little digging, and again knowing almost nothing about c++, I think having a standard definition for units all around probably a really good idea yeah. What I think is happening, is a number of things: in weather.cpp windpower and windspeed are pretty much 1:1, before any modifiers & multipliers. Subsequently, windpower returns windspeed after operation in mph, which is corroborated with the implementation of the Beaufort Scale. In units_utility.cpp, convert_velocity for VU_Wind gets defined for mph-m/s conversion. I have no idea where VU_Wind gets defined, but it's presumably just m/s.

Now, in debug_menu.cpp, the section for setting wind speed adds entries in a for loop in intervals of 10 units. However, these units seem to be defined to be displayed as VU_Wind, which we've seen previously is m/s. The menu is then visually populated with entries 0-100 m/s, and when an entry is selected, that displayed value (through code magic) is stored under weather.windspeed_override, which I think weather.cpp just calls directly to windspeed 1:1, which we've seen previously is defined as mph.

Is there a particular push to move to m/s to use SI units? Otherwise the easiest fix is to change that one line to read mph rather than m/s.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not bump or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.