Time: use locks again

The Atomic implementation was only partially correct, and was very complex.
Use locks for sake of simplicity, following KISS principle.
Only remaining atomic operation use is time of day speed, because that
really is only read + written.

Also fixes a bug with m_time_conversion_skew only being decremented, never
incremented (Regresion from previous commit).

atomic.h changes:
	* Add GenericAtomic<T> class for non-integral types like floats.

	* Remove some last remainders from atomic.h of the volatile use.

src/environment.cpp

@@ -49,11 +49,12 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #define PP(x) "("<<(x).X<<","<<(x).Y<<","<<(x).Z<<")"
 
 Environment::Environment():
+	m_time_of_day_speed(0),
 	m_time_of_day(9000),
 	m_time_of_day_f(9000./24000),
-	m_time_of_day_speed(0),
+	m_time_conversion_skew(0.0f),
-	m_time_conversion_skew(0),
+	m_enable_day_night_ratio_override(false),
-	m_day_night_ratio_override_storage(0)
+	m_day_night_ratio_override(0.0f)
 {
 	m_cache_enable_shaders = g_settings->getBool("enable_shaders");
 }
@@ -179,10 +180,9 @@ std::vector<Player*> Environment::getPlayers(bool ignore_disconnected)
 
 u32 Environment::getDayNightRatio()
 {
-	u64 day_night_st = m_day_night_ratio_override_storage;
+	MutexAutoLock lock(this->m_time_lock);
-	if (day_night_st & ((u64)1 << 63))
+	if (m_enable_day_night_ratio_override)
-		return day_night_st & U32_MAX;
+		return m_day_night_ratio_override;
-	MutexAutoLock lock(this->m_time_floats_lock);
 	return time_to_daynight_ratio(m_time_of_day_f * 24000, m_cache_enable_shaders);
 }
 
@@ -196,29 +196,38 @@ float Environment::getTimeOfDaySpeed()
 	return m_time_of_day_speed;
 }
 
+void Environment::setDayNightRatioOverride(bool enable, u32 value)
+{
+	MutexAutoLock lock(this->m_time_lock);
+	m_enable_day_night_ratio_override = enable;
+	m_day_night_ratio_override = value;
+}
+
 void Environment::setTimeOfDay(u32 time)
 {
-	MutexAutoLock lock(this->m_time_floats_lock);
+	MutexAutoLock lock(this->m_time_lock);
 	m_time_of_day = time;
 	m_time_of_day_f = (float)time / 24000.0;
 }
 
 u32 Environment::getTimeOfDay()
 {
+	MutexAutoLock lock(this->m_time_lock);
 	return m_time_of_day;
 }
 
 float Environment::getTimeOfDayF()
 {
-	MutexAutoLock lock(this->m_time_floats_lock);
+	MutexAutoLock lock(this->m_time_lock);
 	return m_time_of_day_f;
 }
 
 void Environment::stepTimeOfDay(float dtime)
 {
-	MutexAutoLock lock(this->m_time_floats_lock);
+	MutexAutoLock lock(this->m_time_lock);
 	f32 speed = m_time_of_day_speed * 24000. / (24. * 3600);
-	u32 units = (u32)((dtime + m_time_conversion_skew) * speed);
+	m_time_conversion_skew += dtime;
+	u32 units = (u32)(m_time_conversion_skew * speed);
 	bool sync_f = false;
 	if (units > 0) {
 		// Sync at overflow
@@ -232,7 +241,7 @@ void Environment::stepTimeOfDay(float dtime)
 		m_time_conversion_skew -= (f32)units / speed;
 	}
 	if (!sync_f) {
-		m_time_of_day_f += m_time_of_day_speed / (24. * 3600.) * dtime;
+		m_time_of_day_f += speed * dtime;
 		if (m_time_of_day_f > 1.0)
 			m_time_of_day_f -= 1.0;
 		if (m_time_of_day_f < 0.0)
@@ -527,10 +536,10 @@ void ServerEnvironment::loadMeta()
 	}
 
 	try {
-		m_time_of_day = args.getU64("time_of_day");
+		setTimeOfDay(args.getU64("time_of_day"));
 	} catch (SettingNotFoundException &e) {
 		// This is not as important
-		m_time_of_day = 9000;
+		setTimeOfDay(9000);
 	}
 }
 

src/environment.h

@@ -93,10 +93,7 @@ public:
 	void setTimeOfDaySpeed(float speed);
 	float getTimeOfDaySpeed();
 
-	void setDayNightRatioOverride(bool enable, u32 value)
+	void setDayNightRatioOverride(bool enable, u32 value);
-	{
-		m_day_night_ratio_override_storage = value | ((u64)enable << 63);
-	}
 
 	// counter used internally when triggering ABMs
 	u32 m_added_objects;
@@ -105,25 +102,24 @@ protected:
 	// peer_ids in here should be unique, except that there may be many 0s
 	std::vector<Player*> m_players;
 
-	// Time of day in milli-hours (0-23999); determines day and night
+	GenericAtomic<float> m_time_of_day_speed;
-	Atomic<u32> m_time_of_day;
 
 	/*
-	 * Below: values managed by m_time_floats_lock
+	 * Below: values managed by m_time_lock
 	*/
+	// Time of day in milli-hours (0-23999); determines day and night
+	u32 m_time_of_day;
 	// Time of day in 0...1
 	float m_time_of_day_f;
-	float m_time_of_day_speed;
 	// Stores the skew created by the float -> u32 conversion
 	// to be applied at next conversion, so that there is no real skew.
 	float m_time_conversion_skew;
-	/*
-	 * Above: values managed by m_time_floats_lock
-	*/
-
 	// Overriding the day-night ratio is useful for custom sky visuals
-	// lowest 32 bits store the overriden ratio, highest bit stores whether its enabled
+	bool m_enable_day_night_ratio_override;
-	Atomic<u64> m_day_night_ratio_override_storage;
+	u32 m_day_night_ratio_override;
+	/*
+	 * Above: values managed by m_time_lock
+	*/
 
 	/* TODO: Add a callback function so these can be updated when a setting
 	 *       changes.  At this point in time it doesn't matter (e.g. /set
@@ -137,7 +133,7 @@ protected:
 	bool m_cache_enable_shaders;
 
 private:
-	Mutex m_time_floats_lock;
+	Mutex m_time_lock;
 
 	DISABLE_CLASS_COPY(Environment);
 };

src/threading/atomic.h

@@ -24,19 +24,25 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #if __cplusplus >= 201103L
 	#include <atomic>
 	template<typename T> using Atomic = std::atomic<T>;
+	template<typename T> using GenericAtomic = std::atomic<T>;
 #else
 
 #define GCC_VERSION   (__GNUC__        * 100 + __GNUC_MINOR__)
 #define CLANG_VERSION (__clang_major__ * 100 + __clang_minor__)
 #if GCC_VERSION >= 407 || CLANG_VERSION >= 302
+	#define ATOMIC_LOAD_GENERIC(T, v) do {               \
+		T _val;                                          \
+		 __atomic_load(&(v), &(_val), __ATOMIC_SEQ_CST); \
+		return _val;                                     \
+	} while(0)
 	#define ATOMIC_LOAD(T, v)        return __atomic_load_n    (&(v),       __ATOMIC_SEQ_CST)
 	#define ATOMIC_STORE(T, v, x)           __atomic_store     (&(v), &(x), __ATOMIC_SEQ_CST); return x
-	#define ATOMIC_EXCHANGE(T, v, x) return __atomic_exchange_n(&(v), (x),  __ATOMIC_SEQ_CST)
+	#define ATOMIC_EXCHANGE(T, v, x) return __atomic_exchange  (&(v), &(x), __ATOMIC_SEQ_CST)
 	#define ATOMIC_ADD_EQ(T, v, x)   return __atomic_add_fetch (&(v), (x),  __ATOMIC_SEQ_CST)
 	#define ATOMIC_SUB_EQ(T, v, x)   return __atomic_sub_fetch (&(v), (x),  __ATOMIC_SEQ_CST)
 	#define ATOMIC_POST_INC(T, v)    return __atomic_fetch_add (&(v), 1,    __ATOMIC_SEQ_CST)
 	#define ATOMIC_POST_DEC(T, v)    return __atomic_fetch_sub (&(v), 1,    __ATOMIC_SEQ_CST)
-	#define ATOMIC_CAS(T, v, e, d)   return __atomic_compare_exchange_n(&(v), &(e), (d), \
+	#define ATOMIC_CAS(T, v, e, d)   return __atomic_compare_exchange(&(v), &(e), &(d), \
 		false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)
 #else
 	#define ATOMIC_USE_LOCK
@@ -56,12 +62,9 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 			m_mutex.unlock();                \
 			return _eq;                      \
 		} while (0)
-	#define ATOMIC_LOAD(T, v)                     \
+	#define ATOMIC_LOAD(T, v) ATOMIC_LOCK_OP(T, v)
-		if (sizeof(T) <= sizeof(void*)) return v; \
+	#define ATOMIC_LOAD_GENERIC(T, v) ATOMIC_LOAD(T, v)
-		else ATOMIC_LOCK_OP(T, v);
+	#define ATOMIC_STORE(T, v, x) ATOMIC_LOCK_OP(T, v = x)
-	#define ATOMIC_STORE(T, v, x)                     \
-		if (sizeof(T) <= sizeof(void*)) return v = x; \
-		else ATOMIC_LOCK_OP(T, v = x);
 	#define ATOMIC_EXCHANGE(T, v, x) do { \
 			m_mutex.lock();               \
 			T _val = v;                   \
@@ -84,26 +87,48 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 	#endif
 #endif
 
-
+// For usage with integral types.
 template<typename T>
 class Atomic {
 public:
-	Atomic(const T &v = 0) : val(v) {}
+	Atomic(const T &v = 0) : m_val(v) {}
 
-	operator T () { ATOMIC_LOAD(T, val); }
+	operator T () { ATOMIC_LOAD(T, m_val); }
 
-	T exchange(T x)  { ATOMIC_EXCHANGE(T, val, x); }
+	T exchange(T x)  { ATOMIC_EXCHANGE(T, m_val, x); }
-	bool compare_exchange_strong(T &expected, T desired) { ATOMIC_CAS(T, val, expected, desired); }
+	bool compare_exchange_strong(T &expected, T desired) { ATOMIC_CAS(T, m_val, expected, desired); }
 
-	T operator =  (T x) { ATOMIC_STORE(T, val, x); }
+	T operator =  (T x) { ATOMIC_STORE(T, m_val, x); }
-	T operator += (T x) { ATOMIC_ADD_EQ(T, val, x); }
+	T operator += (T x) { ATOMIC_ADD_EQ(T, m_val, x); }
-	T operator -= (T x) { ATOMIC_SUB_EQ(T, val, x); }
+	T operator -= (T x) { ATOMIC_SUB_EQ(T, m_val, x); }
 	T operator ++ ()    { return *this += 1; }
 	T operator -- ()    { return *this -= 1; }
-	T operator ++ (int) { ATOMIC_POST_INC(T, val); }
+	T operator ++ (int) { ATOMIC_POST_INC(T, m_val); }
-	T operator -- (int) { ATOMIC_POST_DEC(T, val); }
+	T operator -- (int) { ATOMIC_POST_DEC(T, m_val); }
 private:
-	T val;
+	T m_val;
+#ifdef ATOMIC_USE_LOCK
+	Mutex m_mutex;
+#endif
+};
+
+// For usage with non-integral types like float for example.
+// Needed because the other operations aren't provided by gcc
+// for non-integral types:
+// https://gcc.gnu.org/onlinedocs/gcc-4.7.0/gcc/_005f_005fatomic-Builtins.html
+template<typename T>
+class GenericAtomic {
+public:
+	GenericAtomic(const T &v = 0) : m_val(v) {}
+
+	operator T () { ATOMIC_LOAD_GENERIC(T, m_val); }
+
+	T exchange(T x)  { ATOMIC_EXCHANGE(T, m_val, x); }
+	bool compare_exchange_strong(T &expected, T desired) { ATOMIC_CAS(T, m_val, expected, desired); }
+
+	T operator = (T x) { ATOMIC_STORE(T, m_val, x); }
+private:
+	T m_val;
 #ifdef ATOMIC_USE_LOCK
 	Mutex m_mutex;
 #endif
@@ -112,4 +137,3 @@ private:
 #endif  // C++11
 
 #endif
-