Fix consistency of sky sun/moon texture behaviour

Also cleans up related code somewhat.

7 files changed, 82 insertions, 142 deletions

diff --git a/doc/lua_api.txt b/doc/lua_api.txt
index 00c29f791..2331736c6 100644
--- a/doc/lua_api.txt
+++ b/doc/lua_api.txt
@@ -6856,7 +6856,7 @@ object you are working with still exists.
         * `visible`: Boolean for whether the sun is visible.
             (default: `true`)
         * `texture`: A regular texture for the sun. Setting to `""`
-            will re-enable the mesh sun. (default: `"sun.png"`)
+            will re-enable the mesh sun. (default: "sun.png", if it exists)
         * `tonemap`: A 512x1 texture containing the tonemap for the sun
             (default: `"sun_tonemap.png"`)
         * `sunrise`: A regular texture for the sunrise texture.
@@ -6872,7 +6872,7 @@ object you are working with still exists.
         * `visible`: Boolean for whether the moon is visible.
             (default: `true`)
         * `texture`: A regular texture for the moon. Setting to `""`
-            will re-enable the mesh moon. (default: `"moon.png"`)
+            will re-enable the mesh moon. (default: "moon.png", if it exists)
         * `tonemap`: A 512x1 texture containing the tonemap for the moon
             (default: `"moon_tonemap.png"`)
         * `scale`: Float controlling the overall size of the moon (default: `1`)
diff --git a/src/client/game.cpp b/src/client/game.cpp
index 182dc3815..b6052390b 100644
--- a/src/client/game.cpp
+++ b/src/client/game.cpp
@@ -2862,7 +2862,7 @@ void Game::handleClientEvent_SetMoon(ClientEvent *event, CameraOrientation *cam)
 void Game::handleClientEvent_SetStars(ClientEvent *event, CameraOrientation *cam)
 {
 	sky->setStarsVisible(event->star_params->visible);
-	sky->setStarCount(event->star_params->count, false);
+	sky->setStarCount(event->star_params->count);
 	sky->setStarColor(event->star_params->starcolor);
 	sky->setStarScale(event->star_params->scale);
 	delete event->star_params;
diff --git a/src/client/sky.cpp b/src/client/sky.cpp
index 1cf9a4afc..0ab710eee 100644
--- a/src/client/sky.cpp
+++ b/src/client/sky.cpp
@@ -18,21 +18,21 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 */
 
+#include <cmath>
 #include "sky.h"
-#include "ITexture.h"
-#include "IVideoDriver.h"
-#include "ISceneManager.h"
-#include "ICameraSceneNode.h"
-#include "S3DVertex.h"
+#include <ITexture.h>
+#include <IVideoDriver.h>
+#include <ISceneManager.h>
+#include <ICameraSceneNode.h>
+#include <S3DVertex.h>
 #include "client/tile.h"
 #include "noise.h" // easeCurve
 #include "profiler.h"
 #include "util/numeric.h"
-#include <cmath>
 #include "client/renderingengine.h"
 #include "settings.h"
 #include "camera.h" // CameraModes
-#include "config.h"
+
 using namespace irr::core;
 
 static video::SMaterial baseMaterial()
@@ -51,7 +51,14 @@ static video::SMaterial baseMaterial()
 	mat.TextureLayer[0].TextureWrapV = video::ETC_CLAMP_TO_EDGE;
 	mat.BackfaceCulling = false;
 	return mat;
-};
+}
+
+static inline void disableTextureFiltering(video::SMaterial &mat)
+{
+	mat.setFlag(video::E_MATERIAL_FLAG::EMF_BILINEAR_FILTER, false);
+	mat.setFlag(video::E_MATERIAL_FLAG::EMF_TRILINEAR_FILTER, false);
+	mat.setFlag(video::E_MATERIAL_FLAG::EMF_ANISOTROPIC_FILTER, false);
+}
 
 Sky::Sky(s32 id, RenderingEngine *rendering_engine, ITextureSource *tsrc, IShaderSource *ssrc) :
 		scene::ISceneNode(rendering_engine->get_scene_manager()->getRootSceneNode(),
@@ -65,6 +72,11 @@ Sky::Sky(s32 id, RenderingEngine *rendering_engine, ITextureSource *tsrc, IShade
 
 	m_enable_shaders = g_settings->getBool("enable_shaders");
 
+	m_sky_params = SkyboxDefaults::getSkyDefaults();
+	m_sun_params = SkyboxDefaults::getSunDefaults();
+	m_moon_params = SkyboxDefaults::getMoonDefaults();
+	m_star_params = SkyboxDefaults::getStarDefaults();
+
 	// Create materials
 
 	m_materials[0] = baseMaterial();
@@ -73,49 +85,15 @@ Sky::Sky(s32 id, RenderingEngine *rendering_engine, ITextureSource *tsrc, IShade
 	m_materials[0].ColorMaterial = video::ECM_NONE;
 
 	m_materials[1] = baseMaterial();
-	//m_materials[1].MaterialType = video::EMT_TRANSPARENT_VERTEX_ALPHA;
 	m_materials[1].MaterialType = video::EMT_TRANSPARENT_ALPHA_CHANNEL;
 
 	m_materials[2] = baseMaterial();
 	m_materials[2].setTexture(0, tsrc->getTextureForMesh("sunrisebg.png"));
 	m_materials[2].MaterialType = video::EMT_TRANSPARENT_ALPHA_CHANNEL;
-	//m_materials[2].MaterialType = video::EMT_TRANSPARENT_ADD_COLOR;
-
-	// Ensures that sun and moon textures and tonemaps are correct.
-	setSkyDefaults();
-	m_sun_texture = tsrc->isKnownSourceImage(m_sun_params.texture) ?
-		tsrc->getTextureForMesh(m_sun_params.texture) : nullptr;
-	m_moon_texture = tsrc->isKnownSourceImage(m_moon_params.texture) ?
-		tsrc->getTextureForMesh(m_moon_params.texture) : nullptr;
-	m_sun_tonemap = tsrc->isKnownSourceImage(m_sun_params.tonemap) ?
-		tsrc->getTexture(m_sun_params.tonemap) : nullptr;
-	m_moon_tonemap = tsrc->isKnownSourceImage(m_moon_params.tonemap) ?
-		tsrc->getTexture(m_moon_params.tonemap) : nullptr;
 
-	if (m_sun_texture) {
-		m_materials[3] = baseMaterial();
-		m_materials[3].setTexture(0, m_sun_texture);
-		m_materials[3].MaterialType = video::EMT_TRANSPARENT_ALPHA_CHANNEL;
-		// Disables texture filtering
-		m_materials[3].setFlag(video::E_MATERIAL_FLAG::EMF_BILINEAR_FILTER, false);
-		m_materials[3].setFlag(video::E_MATERIAL_FLAG::EMF_TRILINEAR_FILTER, false);
-		m_materials[3].setFlag(video::E_MATERIAL_FLAG::EMF_ANISOTROPIC_FILTER, false);
-		// Use tonemaps if available
-		if (m_sun_tonemap)
-			m_materials[3].Lighting = true;
-	}
-	if (m_moon_texture) {
-		m_materials[4] = baseMaterial();
-		m_materials[4].setTexture(0, m_moon_texture);
-		m_materials[4].MaterialType = video::EMT_TRANSPARENT_ALPHA_CHANNEL;
-		// Disables texture filtering
-		m_materials[4].setFlag(video::E_MATERIAL_FLAG::EMF_BILINEAR_FILTER, false);
-		m_materials[4].setFlag(video::E_MATERIAL_FLAG::EMF_TRILINEAR_FILTER, false);
-		m_materials[4].setFlag(video::E_MATERIAL_FLAG::EMF_ANISOTROPIC_FILTER, false);
-		// Use tonemaps if available
-		if (m_moon_tonemap)
-			m_materials[4].Lighting = true;
-	}
+	setSunTexture(m_sun_params.texture, m_sun_params.tonemap, tsrc);
+
+	setMoonTexture(m_moon_params.texture, m_moon_params.tonemap, tsrc);
 
 	for (int i = 5; i < 11; i++) {
 		m_materials[i] = baseMaterial();
@@ -130,7 +108,7 @@ Sky::Sky(s32 id, RenderingEngine *rendering_engine, ITextureSource *tsrc, IShade
 		m_sky_body_orbit_tilt = rangelim(val, 0.0f, 60.0f);
 	}
 
-	setStarCount(1000, true);
+	setStarCount(1000);
 }
 
 void Sky::OnRegisterSceneNode()
@@ -386,20 +364,6 @@ void Sky::update(float time_of_day, float time_brightness,
 
 	bool is_dawn = (time_brightness >= 0.20 && time_brightness < 0.35);
 
-	/*
-	Development colours
-
-	video::SColorf bgcolor_bright_normal_f(170. / 255, 200. / 255, 230. / 255, 1.0);
-	video::SColorf bgcolor_bright_dawn_f(0.666, 200. / 255 * 0.7, 230. / 255 * 0.5, 1.0);
-	video::SColorf bgcolor_bright_dawn_f(0.666, 0.549, 0.220, 1.0);
-	video::SColorf bgcolor_bright_dawn_f(0.666 * 1.2, 0.549 * 1.0, 0.220 * 1.0, 1.0);
-	video::SColorf bgcolor_bright_dawn_f(0.666 * 1.2, 0.549 * 1.0, 0.220 * 1.2, 1.0);
-
-	video::SColorf cloudcolor_bright_dawn_f(1.0, 0.591, 0.4);
-	video::SColorf cloudcolor_bright_dawn_f(1.0, 0.65, 0.44);
-	video::SColorf cloudcolor_bright_dawn_f(1.0, 0.7, 0.5);
-	*/
-
 	video::SColorf bgcolor_bright_normal_f = m_sky_params.sky_color.day_horizon;
 	video::SColorf bgcolor_bright_indoor_f = m_sky_params.sky_color.indoors;
 	video::SColorf bgcolor_bright_dawn_f = m_sky_params.sky_color.dawn_horizon;
@@ -754,33 +718,29 @@ void Sky::setSunTexture(const std::string &sun_texture,
 	// Ignore matching textures (with modifiers) entirely,
 	// but lets at least update the tonemap before hand.
 	m_sun_params.tonemap = sun_tonemap;
-	m_sun_tonemap = tsrc->isKnownSourceImage(m_sun_params.tonemap) ?
-		tsrc->getTexture(m_sun_params.tonemap) : nullptr;
+	m_sun_tonemap = tsrc->isKnownSourceImage(sun_tonemap) ?
+		tsrc->getTexture(sun_tonemap) : nullptr;
 	m_materials[3].Lighting = !!m_sun_tonemap;
 
-	if (m_sun_params.texture == sun_texture)
+	if (m_sun_params.texture == sun_texture && !m_first_update)
 		return;
 	m_sun_params.texture = sun_texture;
 
-	if (sun_texture != "") {
-		// We want to ensure the texture exists first.
-		m_sun_texture = tsrc->getTextureForMesh(m_sun_params.texture);
-
-		if (m_sun_texture) {
-			m_materials[3] = baseMaterial();
-			m_materials[3].setTexture(0, m_sun_texture);
-			m_materials[3].MaterialType = video::
-				EMT_TRANSPARENT_ALPHA_CHANNEL;
-			// Disables texture filtering
-			m_materials[3].setFlag(
-				video::E_MATERIAL_FLAG::EMF_BILINEAR_FILTER, false);
-			m_materials[3].setFlag(
-				video::E_MATERIAL_FLAG::EMF_TRILINEAR_FILTER, false);
-			m_materials[3].setFlag(
-				video::E_MATERIAL_FLAG::EMF_ANISOTROPIC_FILTER, false);
-		}
-	} else {
-		m_sun_texture = nullptr;
+	m_sun_texture = nullptr;
+	if (sun_texture == "sun.png") {
+		// Dumb compatibility fix: sun.png transparently falls back to no texture
+		m_sun_texture = tsrc->isKnownSourceImage(sun_texture) ?
+			tsrc->getTexture(sun_texture) : nullptr;
+	} else if (!sun_texture.empty()) {
+		m_sun_texture = tsrc->getTextureForMesh(sun_texture);
+	}
+
+	if (m_sun_texture) {
+		m_materials[3] = baseMaterial();
+		m_materials[3].setTexture(0, m_sun_texture);
+		m_materials[3].MaterialType = video::EMT_TRANSPARENT_ALPHA_CHANNEL;
+		disableTextureFiltering(m_materials[3]);
+		m_materials[3].Lighting = !!m_sun_tonemap;
 	}
 }
 
@@ -802,40 +762,36 @@ void Sky::setMoonTexture(const std::string &moon_texture,
 	// Ignore matching textures (with modifiers) entirely,
 	// but lets at least update the tonemap before hand.
 	m_moon_params.tonemap = moon_tonemap;
-	m_moon_tonemap = tsrc->isKnownSourceImage(m_moon_params.tonemap) ?
-		tsrc->getTexture(m_moon_params.tonemap) : nullptr;
+	m_moon_tonemap = tsrc->isKnownSourceImage(moon_tonemap) ?
+		tsrc->getTexture(moon_tonemap) : nullptr;
 	m_materials[4].Lighting = !!m_moon_tonemap;
 
-	if (m_moon_params.texture == moon_texture)
+	if (m_moon_params.texture == moon_texture && !m_first_update)
 		return;
 	m_moon_params.texture = moon_texture;
 
-	if (moon_texture != "") {
-		// We want to ensure the texture exists first.
-		m_moon_texture = tsrc->getTextureForMesh(m_moon_params.texture);
-
-		if (m_moon_texture) {
-			m_materials[4] = baseMaterial();
-			m_materials[4].setTexture(0, m_moon_texture);
-			m_materials[4].MaterialType = video::
-				EMT_TRANSPARENT_ALPHA_CHANNEL;
-			// Disables texture filtering
-			m_materials[4].setFlag(
-				video::E_MATERIAL_FLAG::EMF_BILINEAR_FILTER, false);
-			m_materials[4].setFlag(
-				video::E_MATERIAL_FLAG::EMF_TRILINEAR_FILTER, false);
-			m_materials[4].setFlag(
-				video::E_MATERIAL_FLAG::EMF_ANISOTROPIC_FILTER, false);
-		}
-	} else {
-		m_moon_texture = nullptr;
+	m_moon_texture = nullptr;
+	if (moon_texture == "moon.png") {
+		// Dumb compatibility fix: moon.png transparently falls back to no texture
+		m_moon_texture = tsrc->isKnownSourceImage(moon_texture) ?
+			tsrc->getTexture(moon_texture) : nullptr;
+	} else if (!moon_texture.empty()) {
+		m_moon_texture = tsrc->getTextureForMesh(moon_texture);
+	}
+
+	if (m_moon_texture) {
+		m_materials[4] = baseMaterial();
+		m_materials[4].setTexture(0, m_moon_texture);
+		m_materials[4].MaterialType = video::EMT_TRANSPARENT_ALPHA_CHANNEL;
+		disableTextureFiltering(m_materials[4]);
+		m_materials[4].Lighting = !!m_moon_tonemap;
 	}
 }
 
-void Sky::setStarCount(u16 star_count, bool force_update)
+void Sky::setStarCount(u16 star_count)
 {
 	// Allow force updating star count at game init.
-	if (m_star_params.count != star_count || force_update) {
+	if (m_star_params.count != star_count || m_first_update) {
 		m_star_params.count = star_count;
 		updateStars();
 	}
@@ -924,16 +880,6 @@ void Sky::addTextureToSkybox(const std::string &texture, int material_id,
 	m_materials[material_id+5].MaterialType = video::EMT_SOLID;
 }
 
-// To be called once at game init to setup default values.
-void Sky::setSkyDefaults()
-{
-	SkyboxDefaults sky_defaults;
-	m_sky_params.sky_color = sky_defaults.getSkyColorDefaults();
-	m_sun_params = sky_defaults.getSunDefaults();
-	m_moon_params = sky_defaults.getMoonDefaults();
-	m_star_params = sky_defaults.getStarDefaults();
-}
-
 float getWickedTimeOfDay(float time_of_day)
 {
 	float nightlength = 0.415f;
diff --git a/src/client/sky.h b/src/client/sky.h
index 83106453b..3dc057b70 100644
--- a/src/client/sky.h
+++ b/src/client/sky.h
@@ -17,6 +17,8 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 */
 
+#pragma once
+
 #include "irrlichttypes_extrabloated.h"
 #include <ISceneNode.h>
 #include <array>
@@ -25,8 +27,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "shader.h"
 #include "skyparams.h"
 
-#pragma once
-
 #define SKY_MATERIAL_COUNT 12
 
 class ITextureSource;
@@ -77,7 +77,7 @@ public:
 	void setMoonScale(f32 moon_scale) { m_moon_params.scale = moon_scale; }
 
 	void setStarsVisible(bool stars_visible) { m_star_params.visible = stars_visible; }
-	void setStarCount(u16 star_count, bool force_update);
+	void setStarCount(u16 star_count);
 	void setStarColor(video::SColor star_color) { m_star_params.starcolor = star_color; }
 	void setStarScale(f32 star_scale) { m_star_params.scale = star_scale; updateStars(); }
 
@@ -150,7 +150,7 @@ private:
 	bool m_visible = true;
 	// Used when m_visible=false
 	video::SColor m_fallback_bg_color = video::SColor(255, 255, 255, 255);
-	bool m_first_update = true;
+	bool m_first_update = true; // Set before the sky is updated for the first time
 	float m_time_of_day;
 	float m_time_brightness;
 	bool m_sunlight_seen;
@@ -206,7 +206,6 @@ private:
 	void draw_stars(video::IVideoDriver *driver, float wicked_time_of_day);
 	void place_sky_body(std::array<video::S3DVertex, 4> &vertices,
 		float horizon_position,	float day_position);
-	void setSkyDefaults();
 };
 
 // calculates value for sky body positions for the given observed time of day
diff --git a/src/network/clientpackethandler.cpp b/src/network/clientpackethandler.cpp
index f7c586b80..47f259b92 100644
--- a/src/network/clientpackethandler.cpp
+++ b/src/network/clientpackethandler.cpp
@@ -1242,19 +1242,17 @@ void Client::handleCommand_HudSetSky(NetworkPacket* pkt)
 		} catch (...) {}
 
 		// Use default skybox settings:
-		SkyboxDefaults sky_defaults;
-		SunParams sun = sky_defaults.getSunDefaults();
-		MoonParams moon = sky_defaults.getMoonDefaults();
-		StarParams stars = sky_defaults.getStarDefaults();
+		SunParams sun = SkyboxDefaults::getSunDefaults();
+		MoonParams moon = SkyboxDefaults::getMoonDefaults();
+		StarParams stars = SkyboxDefaults::getStarDefaults();
 
 		// Fix for "regular" skies, as color isn't kept:
 		if (skybox.type == "regular") {
-			skybox.sky_color = sky_defaults.getSkyColorDefaults();
+			skybox.sky_color = SkyboxDefaults::getSkyColorDefaults();
 			skybox.fog_tint_type = "default";
 			skybox.fog_moon_tint = video::SColor(255, 255, 255, 255);
 			skybox.fog_sun_tint = video::SColor(255, 255, 255, 255);
-		}
-		else {
+		} else {
 			sun.visible = false;
 			sun.sunrise_visible = false;
 			moon.visible = false;
diff --git a/src/remoteplayer.cpp b/src/remoteplayer.cpp
index 3f0eae0f0..20be7a8c8 100644
--- a/src/remoteplayer.cpp
+++ b/src/remoteplayer.cpp
@@ -31,6 +31,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 /*
 	RemotePlayer
 */
+
 // static config cache for remoteplayer
 bool RemotePlayer::m_setting_cache_loaded = false;
 float RemotePlayer::m_setting_chat_message_limit_per_10sec = 0.0f;
@@ -46,6 +47,7 @@ RemotePlayer::RemotePlayer(const char *name, IItemDefManager *idef):
 			g_settings->getU16("chat_message_limit_trigger_kick");
 		RemotePlayer::m_setting_cache_loaded = true;
 	}
+
 	movement_acceleration_default   = g_settings->getFloat("movement_acceleration_default")   * BS;
 	movement_acceleration_air       = g_settings->getFloat("movement_acceleration_air")       * BS;
 	movement_acceleration_fast      = g_settings->getFloat("movement_acceleration_fast")      * BS;
@@ -59,19 +61,12 @@ RemotePlayer::RemotePlayer(const char *name, IItemDefManager *idef):
 	movement_liquid_sink            = g_settings->getFloat("movement_liquid_sink")            * BS;
 	movement_gravity                = g_settings->getFloat("movement_gravity")                * BS;
 
-	// copy defaults
-	m_cloud_params.density = 0.4f;
-	m_cloud_params.color_bright = video::SColor(229, 240, 240, 255);
-	m_cloud_params.color_ambient = video::SColor(255, 0, 0, 0);
-	m_cloud_params.height = 120.0f;
-	m_cloud_params.thickness = 16.0f;
-	m_cloud_params.speed = v2f(0.0f, -2.0f);
-
 	// Skybox defaults:
+	m_cloud_params  = SkyboxDefaults::getCloudDefaults();
 	m_skybox_params = SkyboxDefaults::getSkyDefaults();
-	m_sun_params = SkyboxDefaults::getSunDefaults();
-	m_moon_params = SkyboxDefaults::getMoonDefaults();
-	m_star_params = SkyboxDefaults::getStarDefaults();
+	m_sun_params    = SkyboxDefaults::getSunDefaults();
+	m_moon_params   = SkyboxDefaults::getMoonDefaults();
+	m_star_params   = SkyboxDefaults::getStarDefaults();
 }
 
 
diff --git a/src/skyparams.h b/src/skyparams.h
index cabbf531c..f7f694427 100644
--- a/src/skyparams.h
+++ b/src/skyparams.h
@@ -82,6 +82,8 @@ struct CloudParams
 class SkyboxDefaults
 {
 public:
+	SkyboxDefaults() = delete;
+
 	static const SkyboxParams getSkyDefaults()
 	{
 		SkyboxParams sky;