widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

> Should we check for g_fs->file_exists here?

No. We expect the images that are loaded here to be there and load_image will throw an error if they are not. If the callsite assumes that the picture is not there, it can catch the error and recover. 

> Why not just return image.get() and do away with the extra variable? Unless image.get() has a side effect that we need for the insert.

It is not the image.get() that has side effects, but the std::move(). 

258	+	images_.insert(make_pair(hash, std::move(image)));

unique_ptr means that there is always exactly one unique_ptr object that owns the underlying raw pointer, i.e. where get() != nullptr. This is necessary, so that the underlying object is deleted only once (when the owning unique_ptr goes out of scope). 
std::move (or assignment) transfers the ownership from one unique_ptr to another, so in our case after calling insert(), image.get == nullptr. so we need to hold on to the pointer object before insert() since we have no way of getting to it after. 

GunChleoc (gunchleoc) wrote 9 minutes ago:
-- 
https://code.launchpad.net/~widelands-dev/widelands/remove_in_memory_image/+merge/243911
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/remove_player_color.