[WIP][CR]Snow Overhaul #28227

davidpwbrown · Feb 16, 2019

Summary

SUMMARY: Features "Adds a proper snow-fall mechanic"

Purpose of change

Make snow an actual thing in-game beyond just a visual change of tiles on day 1 of Winter.

Describe the solution

1.Snow will now fall when the weather is snowing, the game will track how long it snows for and how heavy the snowfall is, and after a certain threshold deep snow piles will accumulate.
2. This snow will slowly fill up the map , it can also melt slowly if the temps go higher.
3. This snow, when settled will cause snow glare when its sunny.
4. This snow , when settled, will be slow to walk through, unless you have snowshoes ( a makeshift item is craftable )
5. Walking in the snow can make your feet cold - if you have no shoes, thats bad, if you have some shoes or mutated feet, its not as bad, if you have waterproof boots, its fine.
6. You can shovel the snow to remove it.
7. You can examine the snow to collect some snow, which can be used in the recipe to make water.
8. Sleeping on the snow would be very cold.
9. You can throw the snowball at zombies, if you want.
10. Someone else could make a recipe for a snowman construction if they wanted.

Problems I need help/advice/feedback on :
1.

The visual aspect of it.
This is tricky, the graphical tiles for... deadpeople tileset for example, that have snow on, ( like t_grass_season_winter )
they do not correspond to any particular terrain.
Instead what happens before this PR, is that when it is winter that tile is automatically selected instead of t_grass.
I may need someone like @SomeDeadGuy or someone familiar with the tilesets , to separate out the various winter tiles, or to help me work out how best to use them to avoid graphical mismatches.
It seems to work passably well at the moment though.

Due to the tricky way seasonal graphical variations are done, I can either make it all ascii fallback, or make it work only on deadpeople tileset, I cant seem to find a solution that works for curses AND all tilesets.

Driving - not in this PR, but I will be looking at traction on snow, and adding snow chains etc.

the Snow item, this is tricky. currently, it dosnt melt. it just sits there, the game does not allow picking up frozen liquids.
I considered doing either a) making it water, then hacking together something that froze it instantly, and filled a container with water( frozen )
or b) making the snow ( frozen ) and then converting it to another item ( water ) when it "melted"
doing b ) would mean probably implementing a whole melting and evaporating code entirely.
doing a) would involve coding picking up frozen objects.

for the time being I went with c) and just made snow a weird item that never melts after you pick it up.
4.

I would like to add trails/footprints in the snow that people/monsters/vehicle leave. not sure how to do that.

I may need pointers and advice on how best to proceed with either of the two options.

I need this to be tested thoroughly.

Ive done as much as I can, but it is so wide-ranging, I am sure I've forgotten something, I'd love people to think about all the things this may affect, mutations, bionics, usages, potential bugs, problems, and list them to me.

Describe alternatives you've considered

Lots

Additional context

Little video of some snow falling whilst waiting ( time sped up )

Night-Pryanik · Feb 17, 2019

You can throw the snowball at zombies

AT LAST!

SomeDeadGuy · Feb 17, 2019

Interesting!
Hit me up on Discord at SomeDeadGuy#3894 if you want help with graphical side of things.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/cata_tiles.cpp

+    if( seasonnum == 2 && map::snowfall > 10000 && !( id == "t_grass" || id == "t_grass_dead" ||
+            id == "t_grass_long" || id == "t_dirt" || id == "t_dirtmound" ) ) {
+        seasonnum = 3;
+    }


Can the whole snippet be considered a temporary hack? The proper way of doing this is by means of the tileset jsons. Meanwhile, it deserves a thorough comment describing why it's here and under what circumstances it can be removed.

This whole thing with the tilesets I'm gonna need to completely re-work, find a much better solution.
The winter versions of trees dont have visible fruit for example.
And it needs to work for tilesets that don't have winter variants either.
I'll come back to this when I think of a better way.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.h

@@ -1299,6 +1300,8 @@ class map
         * If false, monsters are not spawned in view of player character.
         */
        void spawn_monsters( bool ignore_sight );
+        static int snowfall;
+        static int percent_settled;


Why are these here as static members, and not in w_point? They don't seem to belong to map.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/iuse.cpp

+    } else {
+        p->add_msg_if_player( _( "There isn't any snow there." ) );
+        return 0;
+    }


This kind of code is a bit dangerous. moves is mutable and can become uninitialized if someone decides to remove the return statement.

Suggested change

}

if( g->m.furn( pnt ) != f_snow ) {

p->add_msg_if_player( _( "There isn't any snow there." ) );

return 0;

}

const int moves = MINUTES( 120 ) / it->get_quality( DIG ) * 100;

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.cpp

+        snowfall += 8;
+    } else {
+        snowfall -= std::max( 0, ( static_cast<int>( weatherPoint.temperature - 32 ) ) );
+    }


Can be converted to a switch statement inside a constant function that returns increments.

switch( g->weather ) { case WEATHER_FLURRIES: return 2; case WEATHER_SNOW: ... }

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.cpp

+        snowfall -= std::max( 0, ( static_cast<int>( weatherPoint.temperature - 32 ) ) );
+    }
+    snowfall = std::max( 0, snowfall );
+    snowfall = std::min( 172800, snowfall );


There's a clamp function for this.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.cpp

+
+void map::add_snow()
+{
+    if( g->weather >= WEATHER_FLURRIES && snowfall > 3000 ) {


Better be rewritten as a guard clause.

if( g->weather < WEATHER_FLURRIES || snowfall <= 3000 ) { return; }

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.cpp

+        int total_grid = 0;
+        int snowfall_settled = 0;
+        int &gx = gp.x;
+        int &gy = gp.y;


Why this? gp isn't used anywhere else.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.cpp

+        int &gx = gp.x;
+        int &gy = gp.y;
+        int snowfactor = std::min( 15000, snowfall );
+        snowfactor = 15001 - snowfactor;


Can be rewritten as:

const int snowfactor = std::max( 15000 - snowfall, 1 );

Your implementation would yield 15001 when snowfall is 0. Is that intentional?

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.cpp

+        snowfactor = 15001 - snowfactor;
+        for( gx = 0; gx < my_MAPSIZE * SEEX; ++gx ) {
+            for( gy = 0; gy < my_MAPSIZE * SEEY; ++gy ) {
+                total_grid += 1;


There's no need to increment here. This value can be calculated in compile time outside the loops as my_MAPSIZE * SEEX * my_MAPSIZE * SEEY

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.cpp

+                if( ( !ter( map_location )->has_flag( TFLAG_INDOORS ) ) &&
+                    ( !ter( map_location )->has_flag( "TREE" ) ) && ( is_outside( map_location ) ) &&
+                    ( furn( map_location ) == f_null ) ) {
+                    if( one_in( std::max( 2, static_cast<int>( snowfactor / 2 ) ) ) ) {


std::max( 2, static_cast<int>( snowfactor / 2 ) ) can be extracted from the loop as well.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.cpp

+                    ( furn( map_location ) == f_null ) ) {
+                    if( one_in( std::max( 2, static_cast<int>( snowfactor / 2 ) ) ) ) {
+                        furn_set( map_location, f_snow );
+                        snowfall_settled += 1;


++snowfall_settled would be more conventional.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.cpp

+                }
+            }
+        }
+        percent_settled = std::min( 100, static_cast<int>( ( snowfall_settled / total_grid ) * 100 ) );


snowfall_settled increments only when there was no snow on the tile initially and it was randomly placed there. Which means it denotes the amount of snow placed per invocation of add_snow, not the actual ratio. This seems to be wrong.

snowfall_settled / total_grid isn't what you would expect. Both are integers and produce either 0 or 1 (nothing else in the current implementation). Should be static_cast<float>(snowfall_settled) / total_grid to produce decimals.

std::min( 100, ...) is excessive, the ratio never exceeds 1.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.cpp

+                }
+            }
+        }
+        percent_settled = std::max( 0, static_cast<int>( ( snowfall_settled / total_grid ) * 100 ) );


Same here. Also, these functions look quite alike. Can their guts be deduplicated?

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/iexamine.cpp

+        p.i_add( item( "snow" ) );
+        p.add_msg_if_player( _( "You gathered some snow" ) );
+    } else {
+        g->m.add_item_or_charges( p.pos(), item( "snow" ) );


item gets constructed four times (isn't quite cheap) from the same string literal. I'd extract a temporary for both deduplication and performance.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/iexamine.cpp

@@ -3044,6 +3045,22 @@ void iexamine::water_source(player &p, const tripoint &examp)
    g->handle_liquid( water, nullptr, 0, &examp );
 }

+void iexamine::gather_snow(player &p, const tripoint &examp)
+{
+    if(!query_yn(_("Gather snow?"))) {


Coding style.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.cpp

+            if( ( !tmpsub->get_ter( p ).obj().has_flag( TFLAG_INDOORS ) ) &&
+                ( !tmpsub->get_ter( p ).obj().has_flag( "TREE" ) )  &&
+                ( tmpsub->get_furn( p ) == f_null ) && ( is_outside( pnt ) ) &&
+                x_in_y( percent_settled, 100 ) ) {


Excessive parenthesis.

has_flag are presumably the most expensive checks here, they should go last.

obj() can be omitted, simply get_ter( p )->has_flag( TFLAG_INDOORS ) would work too.

Not quite sure about the "TREE" flag (looks like a hack which doesn't cover all possible cases).

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/map.cpp

+                ( !tmpsub->get_ter( p ).obj().has_flag( "TREE" ) )  &&
+                ( tmpsub->get_furn( p ) == f_null ) && ( is_outside( pnt ) ) &&
+                x_in_y( percent_settled, 100 ) ) {
+                tmpsub->frn[i][j] = furn_id( "f_snow" );


Please use set_furn() instead. It has an important side-effect (is_uniform = false;). Low-level code is best avoided unless there's a strong reason.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/player.h

@@ -1744,6 +1744,7 @@ class player : public Character

        /** Can the player lie down and cover self with blankets etc. **/
        bool can_use_floor_warmth() const;
+        bool walking_over_snow() const;


This function bloats the player class. I'd suggest getting rid of it.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/weather.cpp

    }
-
+    data.averagetemperature = static_cast<int>( data.averagetemperature / count );


This code is also suspicious: data.averagetemperature / count follows the integer division rules and can't possibly have a fractional part, casting to int is excessive, it's already an int. You might want static_cast<float>(data.averagetemperature) / count or averagetemperature being float initially.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/player.cpp

@@ -1042,6 +1042,7 @@ void player::update_bodytemp()
    const bool has_climate_control = in_climate_control();
    const bool use_floor_warmth = can_use_floor_warmth();
    const furn_id furn_at_pos = g->m.furn( pos() );
+    const bool snowonground = walking_over_snow();


This temp is declared way too far from the place of utilization.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/player.cpp

+            if (worn_item.covers(bp) && worn_item.has_flag("WATERPROOF") ){
+                waterproof_shoes = true;
+            }
+        }


There's the worn_with_flag function for this.

codemime · Feb 17, 2019

codemime reviewed Feb 17, 2019

View changes

codemime · Feb 17, 2019

src/player.cpp

+                        temp_conv[bp] -= 450;
+                    }
+                }
+                break;


Duplicated cases:

case bp_foot_l: case bp_foot_r: ...

You probably don't want a gigantic switch statement here since you're only interested in feet. A couple of conditions in an if would suffice.

davidpwbrown · Feb 18, 2019

Yeah, i'ma fix up the horrible code at some point, I'm trying to think of ways to solve the difficult problems of the visuals/tiles. then I'll fix it up the rest..

Throwaway-name · Mar 8, 2019

Would this work with this?: #27387

davidpwbrown · Mar 12, 2019

Would this work with this?: #27387

Possibly. eventually I want to track water on the ground from rainwater, and the thermal energy system could potentially be used to freeze puddles/rivers and let snow settle. I'll come back to this and similar things when I get better ideas for how to implement it.

davidpwbrown · Mar 29, 2019

a project for another time. perhaps.

kevingranade · Jun 29, 2019

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/winter-activities-melting-snow-for-water-snow-taffy-building-snowmen-throwing-snowballs/20579/6

davidpwbrown added some commits Feb 15, 2019

initial snowfall
initial WIP version initial snowfall initial WIP version

9de9206

modified tileset

6bd69c0

removed test edit

Loading status checks…

b08cd54

revert tile_config

Loading status checks…

50ad04f

davidpwbrown referenced this pull request Feb 16, 2019
Closed
Fixed snowglare at night #28213

Night-Pryanik added <Enhancement / Feature> Weather 0.D Freeze labels Feb 17, 2019

kevingranade added this to Candidate Features in 0.E Release Feb 18, 2019

I-am-Erk moved this from Candidate to Short-list Candidate in 0.E Release Feb 22, 2019

I-am-Erk moved this from Non-vote Candidate (Maintainer's Choice) to Candidate in 0.E Release Feb 22, 2019

kevingranade force-pushed the CleverRaven:master branch from f07c447 to c5d1def Feb 25, 2019

davidpwbrown referenced this pull request Feb 25, 2019
Closed
Remove Snowglare #28371

kevingranade changed the base branch from master to development Feb 25, 2019

ifreund removed the 0.D Freeze label Mar 8, 2019

ZhilkinSerg changed the base branch from development to master Mar 21, 2019

davidpwbrown closed this Mar 29, 2019

0.E Release automation moved this from Voted Off the Island (can still be personal projects obvs) to Done Mar 29, 2019

davidpwbrown deleted the davidpwbrown:snow_accum branch Apr 2, 2019

CleverRaven/Cataclysm-DDA

Join GitHub today

[WIP][CR]Snow Overhaul #28227

Conversation

davidpwbrown commented Feb 16, 2019 • edited

Summary

Purpose of change

Describe the solution

Describe alternatives you've considered

Additional context

davidpwbrown added some commits Feb 15, 2019

davidpwbrown referenced this pull request Feb 16, 2019

Night-Pryanik added <Enhancement / Feature> Weather 0.D Freeze labels Feb 17, 2019

This comment has been minimized.

Night-Pryanik commented Feb 17, 2019

This comment has been minimized.

SomeDeadGuy commented Feb 17, 2019

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime Feb 17, 2019 • edited

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

codemime reviewed Feb 17, 2019 View changes

This comment has been minimized.

This comment has been minimized.

davidpwbrown commented Feb 18, 2019 • edited

kevingranade added this to Candidate Features in 0.E Release Feb 18, 2019

I-am-Erk moved this from Candidate to Short-list Candidate in 0.E Release Feb 22, 2019

I-am-Erk moved this from Non-vote Candidate (Maintainer's Choice) to Candidate in 0.E Release Feb 22, 2019

kevingranade force-pushed the CleverRaven:master branch from f07c447 to c5d1def Feb 25, 2019

davidpwbrown referenced this pull request Feb 25, 2019

kevingranade changed the base branch from master to development Feb 25, 2019

ifreund removed the 0.D Freeze label Mar 8, 2019

This comment has been minimized.

Throwaway-name commented Mar 8, 2019

This comment has been minimized.

davidpwbrown commented Mar 12, 2019

ZhilkinSerg changed the base branch from development to master Mar 21, 2019

This comment has been minimized.

davidpwbrown commented Mar 29, 2019

davidpwbrown closed this Mar 29, 2019

davidpwbrown commented Feb 16, 2019 •

edited

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime Feb 17, 2019 •

edited

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

codemime reviewed Feb 17, 2019

View changes

davidpwbrown commented Feb 18, 2019 •

edited

kevingranade force-pushed the CleverRaven:master branch from `f07c447` to `c5d1def` Feb 25, 2019