ryzom / ryzomcore

Ryzom Core is the open-source project related to the Ryzom game. This community repository is synchronized with the Ryzom Forge repository, based on the Core branch.
https://wiki.ryzom.dev
GNU Affero General Public License v3.0
341 stars 90 forks source link

different weather on client and server #231

Closed ryzom-pipeline closed 7 years ago

ryzom-pipeline commented 9 years ago

Original report by Meelis Mägi (Bitbucket: [Meelis Mägi](https://bitbucket.org/Meelis Mägi), ).


Before WITH_SSE3 addition it was simple:

but now with the sse3 turned on it's a bit more complicated

this is linux client. dont have other platforms to test. compiler is gcc (Ubuntu 4.8.2-19ubuntu1)

So whos going to teach server to seed client with CNoiseValue values?

ryzom-pipeline commented 9 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


But what's causing the difference?

ryzom-pipeline commented 9 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


Also, this is a nice read http://blogs.unity3d.com/2015/01/07/a-primer-on-repeatable-random-numbers/

ryzom-pipeline commented 9 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


Tbh, for fixing this, I don't care very much about retaining backwards compatibility. As long as it functions in a comparable manner, from a leveldesign point of view, and works the same regardless of compiler or platform, it can be adopted, and we should just move away from whatever it's doing right now.

The only method I would not agree with, would be to send extra values over the network. That would be an unnecessary hack. I would prefer to see a reliable noise method that works the same everywhere.

ryzom-pipeline commented 9 years ago

Original comment by Meelis Mägi (Bitbucket: [Meelis Mägi](https://bitbucket.org/Meelis Mägi), ).


heavy use of floating point math is causing the difference. rounding error creeps in on the very first operation and is multiplied with next ones.

Using integer hash function seems to give comparable result with CNoiseValue.

Here is how it would appear in game, top (blue) is CNoiseValue, bottom (red) is using hash, day lines are vertical and weather condition is horizontal. best is sunny, worst is thunderstorm.

Patch is easy and works the same for 32bit and 64bit builds

edit: updated patch below

ryzom-pipeline commented 9 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


Cool. :D

ryzom-pipeline commented 9 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


How about changing

float noiseValue = (float)val / 0xffffffff;
uint32 value = (uint32) (noiseValue * (float) wf.getWeatherSetupsTotalWeight());

To something like

uint32 divweight = 0xffffffff / wf.getWeatherSetupsTotalWeight();
uint32 value = val / divweight;

?

ryzom-pipeline commented 9 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


Or even simpler

uint32 value = val % wf.getWeatherSetupsTotalWeight();

Unless I'm misreading things somewhere.

ryzom-pipeline commented 9 years ago

Original comment by Meelis Mägi (Bitbucket: [Meelis Mägi](https://bitbucket.org/Meelis Mägi), ).


yep, getting rid of floats in there works.

updated patch,

diff -r 005c389aef13 code/ryzom/common/src/game_share/time_weather_season/weather_predict.cpp
--- a/code/ryzom/common/src/game_share/time_weather_season/weather_predict.cpp  Mon Mar 02 13:13:45 2015 +0100
+++ b/code/ryzom/common/src/game_share/time_weather_season/weather_predict.cpp  Sat Mar 07 00:12:51 2015 +0200
@@ -27,24 +27,23 @@
 //
 #include "weather_function_params_sheet_base.h"
 #include "nel/misc/algo.h"
-#include "nel/misc/random.h"
-#include "nel/misc/noise_value.h"
-#include "nel/misc/fast_floor.h"
 //

-
-static NLMISC::CNoiseValue nv;
-
 float CPredictWeather::getCycleWeatherValue(uint64 cycle, const CWeatherFunction &wf)
 {
        uint numWS = wf.getNumWeatherSetups();
-       if (!numWS) return 0.f;
-       NLMISC::CRandom rnd;
-       NLMISC::OptFastFloorBegin();
-       float noiseValue = nv.eval(NLMISC::CVector(cycle * 0.99524f, cycle * 0.85422f, cycle * -0.45722f));
-       NLMISC::OptFastFloorEnd();
-       noiseValue = fmodf(noiseValue * 10.f, 1.f); // make distribution more uniform
-       uint32 value = (uint32) (noiseValue * (float) wf.getWeatherSetupsTotalWeight());
+       uint32 totalWeight = wf.getWeatherSetupsTotalWeight();
+       if (!numWS || totalWeight == 0) return 0.f;
+
+       // Based on Thomas Wang’s integer hash function
+       uint32 hash = (cycle ^ 61) ^ (cycle >> 16);
+       hash = hash + (hash << 3);
+       hash = hash ^ (hash >> 4);
+       hash *= 0x27d4eb2d;
+       hash = hash ^ (hash >> 15);
+
+       // (hash/uint32max) * totalWeight
+       uint32 value = hash / (0xffffffff / totalWeight);
        uint32 currWeight = 0;
        for(uint k = 0; k < numWS; ++k)
        {
ryzom-pipeline commented 9 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


The % version should probably work too then. One division less too.

So, if I understand this correctly, the original code was just abusing the 3D noise function to generate a "consistent" integer purely random number that is selected by the cycle integer?

ryzom-pipeline commented 9 years ago

Original comment by Meelis Mägi (Bitbucket: [Meelis Mägi](https://bitbucket.org/Meelis Mägi), ).


yes % seems to be good enough too.

CNoiseValue should give more natural values or something like that.

ryzom-pipeline commented 9 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


Yeah, but since it's doing

noiseValue = fmodf(noiseValue * 10.f, 1.f); // make distribution more uniform

It essentially seems to just destroy any of that naturalness in favour of a uniform random

ryzom-pipeline commented 9 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


Added the wang hash function to a new separate header. Simplifies the code for this a bit.

https://bitbucket.org/ryzom/ryzomcore/commits/55ed5aaa6a340951e3b9ff564e727b1aa8a6a90c

ryzom-pipeline commented 9 years ago

Original comment by Cédric Ochs (Bitbucket: [Cédric OCHS](https://bitbucket.org/Cédric OCHS), ).


Please something changed about that ?

I think it's more important to be fixed in Ryzom compatibility branches than Ryzom Core ones :)

ryzom-pipeline commented 9 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


I was waiting with this since this needs to be released on both server and client at the same time as this does change the weather on both (so technically it 'breaks' ryzom game).

There's no problem putting it into Ryzom Core.

ryzom-pipeline commented 9 years ago

Original comment by Cédric Ochs (Bitbucket: [Cédric OCHS](https://bitbucket.org/Cédric OCHS), ).


Cool, once it will be commited I'll apply it in private code :) In fact, WG agreed to use Ryzom Core repository for all clients (even Windows one), while servers are still compiled with private code (and using Nevrax Makefiles...).

ryzom-pipeline commented 8 years ago

Original comment by Cédric Ochs (Bitbucket: [Cédric OCHS](https://bitbucket.org/Cédric OCHS), ).


I think this fix didn't work as expected, I still have storms in summer on Yubo (the patch has been applied).

ryzom-pipeline commented 8 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


I don't think there's anything wrong with storms in summer?

This only fixes difference between server and client randomization.

ryzom-pipeline commented 8 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


<STRUCT Name="SummerWeatherFunction">
  <ARRAY Name="WeatherSetups">
    <ATOM Value="de_fair1.weather_setup"/>
    <ATOM Value="de_fair2.weather_setup"/>
    <ATOM Value="de_fair3.weather_setup"/>
    <ATOM Value="de_clouds1.weather_setup"/>
    <ATOM Value="de_clouds2.weather_setup"/>
    <ATOM Value="de_thunder_sand.weather_setup"/>
  </ARRAY>
  <ARRAY Name="SetupsWeights">
    <ATOM Value="50"/>
    <ATOM Value="30"/>
    <ATOM Value="10"/>
    <ATOM Value="3"/>
    <ATOM Value="3"/>
    <ATOM Value="4"/>
  </ARRAY>
</STRUCT>
ryzom-pipeline commented 8 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


<STRUCT Name="SummerWeatherFunction">
  <ARRAY Name="WeatherSetups">
    <ATOM Value="fo_fair1.weather_setup"/>
    <ATOM Value="fo_fair2.weather_setup"/>
    <ATOM Value="fo_fair3.weather_setup"/>
    <ATOM Value="fo_clouds1.weather_setup"/>
    <ATOM Value="fo_clouds2.weather_setup"/>
    <ATOM Value="fo_thunder_storm.weather_setup"/>
  </ARRAY>
  <ARRAY Name="SetupsWeights">
    <ATOM Value="40"/>
    <ATOM Value="20"/>
    <ATOM Value="20"/>
    <ATOM Value="10"/>
    <ATOM Value="5"/>
    <ATOM Value="5"/>
  </ARRAY>
</STRUCT>
ryzom-pipeline commented 8 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


So there's about 5% chance of storm during summer ;)

ryzom-pipeline commented 8 years ago

Original comment by Cédric Ochs (Bitbucket: [Cédric OCHS](https://bitbucket.org/Cédric OCHS), ).


Hum, it seems they are there 100% of time :p I'll check again.

ryzom-pipeline commented 8 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


Hmm.. I'll try checking the weather function this week, there's some other thing in there I want to verify anyway. I think there's some floating point error accumulation somewhere.

ryzom-pipeline commented 8 years ago

Original comment by Meelis Mägi (Bitbucket: [Meelis Mägi](https://bitbucket.org/Meelis Mägi), ).


Client can draw ingame weather graph. client/src/main_loop.cpp and search for DisplayWeatherFunction

ryzom-pipeline commented 7 years ago

Original comment by Guillaume DUPUY (Bitbucket: [Guillaume DUPUY](https://bitbucket.org/Guillaume DUPUY), ).


Isn't this fixed now ? Client weather does seems to always follow server weather, as far as I can tell !

ryzom-pipeline commented 7 years ago

Original comment by Meelis Mägi (Bitbucket: [Meelis Mägi](https://bitbucket.org/Meelis Mägi), ).


thanks for reminding, yes its on live servers