Skip to content
Permalink
Browse files
Cpp11 initializers 2 (#5999)
* C++11 patchset 10: continue cleanup on constructors

* Drop obsolete bool MainMenuData::enable_public (setting is called with cURL in server loop)

* More classes cleanup

* More classes cleanup + change NULL tests to boolean tests
  • Loading branch information
nerzhul committed Jun 17, 2017
1 parent 76be103 commit 8f7785771b9e02b1a1daf7a252550d78ea93053d
Show file tree
Hide file tree
Showing 59 changed files with 333 additions and 646 deletions.
@@ -221,7 +221,7 @@ Client::~Client()
scene::IAnimatedMesh *mesh =
m_device->getSceneManager()->getMeshCache()->getMeshByIndex(0);

if (mesh != NULL)
if (mesh)
m_device->getSceneManager()->getMeshCache()->removeMesh(mesh);
}

@@ -383,7 +383,7 @@ void Client::step(float dtime)
*/
// Control local player (0ms)
LocalPlayer *player = m_env.getLocalPlayer();
assert(player != NULL);
assert(player);
player->applyControl(dtime);

// Step environment
@@ -459,7 +459,7 @@ void Client::step(float dtime)
if (block) {
// Delete the old mesh
delete block->mesh;
block->mesh = NULL;
block->mesh = nullptr;

if (r.mesh) {
minimap_mapblock = r.mesh->moveMinimapMapblock();
@@ -1370,7 +1370,7 @@ void Client::addNode(v3s16 p, MapNode n, bool remove_metadata)
void Client::setPlayerControl(PlayerControl &control)
{
LocalPlayer *player = m_env.getLocalPlayer();
assert(player != NULL);
assert(player);
player->control = control;
}

@@ -1394,7 +1394,7 @@ bool Client::getLocalInventoryUpdated()
void Client::getLocalInventory(Inventory &dst)
{
LocalPlayer *player = m_env.getLocalPlayer();
assert(player != NULL);
assert(player);
dst = player->inventory;
}

@@ -1407,7 +1407,7 @@ Inventory* Client::getInventory(const InventoryLocation &loc)
case InventoryLocation::CURRENT_PLAYER:
{
LocalPlayer *player = m_env.getLocalPlayer();
assert(player != NULL);
assert(player);
return &player->inventory;
}
break;
@@ -1496,7 +1496,7 @@ void Client::setCrack(int level, v3s16 pos)
u16 Client::getHP()
{
LocalPlayer *player = m_env.getLocalPlayer();
assert(player != NULL);
assert(player);
return player->hp;
}

@@ -1529,7 +1529,7 @@ void Client::typeChatMessage(const std::wstring &message)
// compatibility code
if (m_proto_ver < 29) {
LocalPlayer *player = m_env.getLocalPlayer();
assert(player != NULL);
assert(player);
std::wstring name = narrow_to_wide(player->getName());
pushToChatQueue((std::wstring)L"<" + name + L"> " + message);
}
@@ -457,7 +457,7 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef
bool nodedefReceived()
{ return m_nodedef_received; }
bool mediaReceived()
{ return m_media_downloader == NULL; }
{ return !m_media_downloader; }

u8 getProtoVersion()
{ return m_proto_ver; }
@@ -357,8 +357,6 @@ bool ClientLauncher::launch_game(std::string &error_message,
if (cmd_args.exists("password"))
menudata.password = cmd_args.get("password");

menudata.enable_public = g_settings->getBool("server_announce");

// If a world was commanded, append and select it
if (game_params.world_path != "") {
worldspec.gameid = getWorldGameId(game_params.world_path, true);
@@ -79,11 +79,11 @@ void RemoteClient::GetNextBlocks (

RemotePlayer *player = env->getPlayer(peer_id);
// This can happen sometimes; clients and players are not in perfect sync.
if (player == NULL)
if (!player)
return;

PlayerSAO *sao = player->getPlayerSAO();
if (sao == NULL)
if (!sao)
return;

// Won't send anything if already sending
@@ -275,8 +275,7 @@ void RemoteClient::GetNextBlocks (

bool surely_not_found_on_disk = false;
bool block_is_invalid = false;
if(block != NULL)
{
if (block) {
// Reset usage timer, this block will be of use in the future.
block->resetUsageTimer();

@@ -645,7 +644,7 @@ void ClientInterface::step(float dtime)

void ClientInterface::UpdatePlayerList()
{
if (m_env != NULL) {
if (m_env) {
std::vector<u16> clients = getClientIDs();
m_clients_names.clear();

@@ -664,7 +663,7 @@ void ClientInterface::UpdatePlayerList()
{
MutexAutoLock clientslock(m_clients_mutex);
RemoteClient* client = lockedGetClientNoEx(*i);
if(client != NULL)
if (client)
client->PrintInfo(infostream);
}

@@ -212,11 +212,11 @@ void ClientMap::updateDrawList(video::IVideoDriver* driver)
if not seen on display
*/

if (block->mesh != NULL)
if (block->mesh)
block->mesh->updateCameraOffset(m_camera_offset);

float range = 100000 * BS;
if (m_control.range_all == false)
if (!m_control.range_all)
range = m_control.wanted_range * BS;

float d = 0.0;
@@ -229,7 +229,7 @@ void ClientMap::updateDrawList(video::IVideoDriver* driver)
/*
Ignore if mesh doesn't exist
*/
if (block->mesh == NULL) {
if (!block->mesh) {
blocks_in_range_without_mesh++;
continue;
}
@@ -398,7 +398,7 @@ void ClientMap::renderMap(video::IVideoDriver* driver, s32 pass)
MapBlock *block = i->second;

// If the mesh of the block happened to get deleted, ignore it
if (block->mesh == NULL)
if (!block->mesh)
continue;

float d = 0.0;
@@ -202,7 +202,7 @@ void TestCAO::addToScene(scene::ISceneManager *smgr, ITextureSource *tsrc,

void TestCAO::removeFromScene(bool permanent)
{
if(m_node == NULL)
if (!m_node)
return;

m_node->remove();
@@ -220,7 +220,7 @@ v3s16 TestCAO::getLightPosition()

void TestCAO::updateNodePos()
{
if(m_node == NULL)
if (!m_node)
return;

m_node->setPosition(m_position);
@@ -377,16 +377,16 @@ void ItemCAO::addToScene(scene::ISceneManager *smgr, ITextureSource *tsrc,

void ItemCAO::removeFromScene(bool permanent)
{
if(m_node == NULL)
if (!m_node)
return;

m_node->remove();
m_node = NULL;
m_node = nullptr;
}

void ItemCAO::updateLight(u8 light_at_pos)
{
if(m_node == NULL)
if (!m_node)
return;

u8 li = decode_light(light_at_pos);
@@ -401,7 +401,7 @@ v3s16 ItemCAO::getLightPosition()

void ItemCAO::updateNodePos()
{
if(m_node == NULL)
if (!m_node)
return;

m_node->setPosition(m_position);
@@ -428,7 +428,7 @@ void ItemCAO::updateInfoText()

void ItemCAO::updateTexture()
{
if(m_node == NULL)
if (!m_node)
return;

// Create an inventory item to see what is its image
@@ -1155,13 +1155,13 @@ void GenericCAO::step(float dtime, ClientEnvironment *env)
updateTextures(m_previous_texture_modifier);
}
}
if(getParent() == NULL && fabs(m_prop.automatic_rotate) > 0.001)
if(!getParent() && fabs(m_prop.automatic_rotate) > 0.001)
{
m_yaw += dtime * m_prop.automatic_rotate * 180 / M_PI;
updateNodePos();
}

if (getParent() == NULL && m_prop.automatic_face_movement_dir &&
if (!getParent() && m_prop.automatic_face_movement_dir &&
(fabs(m_velocity.Z) > 0.001 || fabs(m_velocity.X) > 0.001))
{
float optimal_yaw = atan2(m_velocity.Z,m_velocity.X) * 180 / M_PI
@@ -1408,7 +1408,7 @@ void GenericCAO::updateTextures(std::string mod)

void GenericCAO::updateAnimation()
{
if(m_animated_meshnode == NULL)
if (!m_animated_meshnode)
return;

if (m_animated_meshnode->getStartFrame() != m_animation_range.X ||
@@ -1426,7 +1426,7 @@ void GenericCAO::updateAnimation()

void GenericCAO::updateBonePosition()
{
if(m_bone_position.empty() || m_animated_meshnode == NULL)
if(m_bone_position.empty() || !m_animated_meshnode)
return;

m_animated_meshnode->setJointMode(irr::scene::EJUOR_CONTROL); // To write positions to the mesh on render
@@ -1447,7 +1447,7 @@ void GenericCAO::updateBonePosition()
void GenericCAO::updateAttachments()
{

if (getParent() == NULL) { // Detach or don't attach
if (!getParent()) { // Detach or don't attach
scene::ISceneNode *node = getSceneNode();
if (node) {
v3f old_position = node->getAbsolutePosition();
@@ -28,9 +28,9 @@ with this program; if not, write to the Free Software Foundation, Inc.,


Environment::Environment(IGameDef *gamedef):
m_gamedef(gamedef),
m_time_of_day_speed(0.0f),
m_day_count(0)
m_day_count(0),
m_gamedef(gamedef)
{
m_cache_enable_shaders = g_settings->getBool("enable_shaders");
m_cache_active_block_mgmt_interval = g_settings->getFloat("active_block_mgmt_interval");
@@ -2643,7 +2643,7 @@ void Game::openInventory()
*/

LocalPlayer *player = client->getEnv().getLocalPlayer();
if (player == NULL || player->getCAO() == NULL)
if (!player || !player->getCAO())
return;

infostream << "the_game: " << "Launching inventory" << std::endl;
@@ -3402,7 +3402,7 @@ void Game::updateCamera(u32 busy_time, f32 dtime)
GenericCAO *playercao = player->getCAO();

// If playercao not loaded, don't change camera
if (playercao == NULL)
if (!playercao)
return;

camera->toggleCameraMode();
@@ -76,12 +76,9 @@ GUIChatConsole::GUIChatConsole(

m_font = g_fontengine->getFont(FONT_SIZE_UNSPECIFIED, FM_Mono);

if (m_font == NULL)
{
if (!m_font) {
errorstream << "GUIChatConsole: Unable to load mono font ";
}
else
{
} else {
core::dimension2d<u32> dim = m_font->getDimension(L"M");
m_fontsize = v2u32(dim.Width, dim.Height);
m_font->grab();
@@ -353,7 +350,7 @@ void GUIChatConsole::drawText()

void GUIChatConsole::drawPrompt()
{
if (m_font == NULL)
if (!m_font)
return;

u32 row = m_chat_backend->getConsoleBuffer().getRows();
@@ -323,7 +323,7 @@ GUIEngine::~GUIEngine()

//clean up texture pointers
for (unsigned int i = 0; i < TEX_LAYER_MAX; i++) {
if (m_textures[i].texture != NULL)
if (m_textures[i].texture)
driver->removeTexture(m_textures[i].texture);
}

@@ -2004,7 +2004,7 @@ void GUIFormSpecMenu::regenerateGui(v2u32 screensize)

// Add tooltip
{
assert(m_tooltip_element == NULL);
assert(!m_tooltip_element);
// Note: parent != this so that the tooltip isn't clipped by the menu rectangle
m_tooltip_element = addStaticText(Environment, L"",core::rect<s32>(0,0,110,18));
m_tooltip_element->enableOverrideColor(true);
@@ -2157,7 +2157,7 @@ void GUIFormSpecMenu::regenerateGui(v2u32 screensize)
m_tooltip_element->setOverrideFont(m_font);

gui::IGUISkin* skin = Environment->getSkin();
sanity_check(skin != NULL);
sanity_check(skin);
gui::IGUIFont *old_font = skin->getFont();
skin->setFont(m_font);

5 comments on commit 8f77857

@paramat
Copy link
Contributor

@paramat paramat commented on 8f77857 Jun 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

 MapgenFlatParams::MapgenFlatParams()
 {
-	spflags          = 0;
-	ground_level     = 8;
-	large_cave_depth = -33;
-	cave_width       = 0.09;
-	lake_threshold   = -0.45;
-	lake_steepness   = 48.0;
-	hill_threshold   = 0.45;
-	hill_steepness   = 64.0;

@nerzhul this is a bit of a shock, how do i decide whether a parameter can remain here or not? Why do the noise parameters remain here?

@nerzhul
Copy link
Member Author

@nerzhul nerzhul commented on 8f77857 Jun 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paramat look at the header file directly

-	u32 spflags;
-	s16 ground_level;
-	s16 large_cave_depth;
-	float cave_width;
-	float lake_threshold;
-	float lake_steepness;
-	float hill_threshold;
-	float hill_steepness;
+	u32 spflags = 0;
+	s16 ground_level = 8;
+	s16 large_cave_depth = -33;
+	float cave_width = 0.09f;
+	float lake_threshold = -0.45f;
+	float lake_steepness = 48.0f;
+	float hill_threshold = 0.45f;
+	float hill_steepness = 64.0f;

in C++11 all trivial types defaults should be defined with membler declaration. You can do that for noise params too if needed, i just moved trivial types.

@paramat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok thanks.

@paramat
Copy link
Contributor

@paramat paramat commented on 8f77857 Jul 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nerzhul i find it very useful when workng on the 7 mapgens to have all mapgen parameters in one place and just above the parameter getters and setters in the .cpp file, please can i move these simple parameters back to the .cpp file?

I don't mind simple parameters being moved anywhere else in the code, just in the files of the 7 core mapgens.

I considered moving the noise parameters to the .h file but the addition of 'NoiseParams' to each line will make the lines excessively long and they will become less unreadable (for me) if i split the lines.

@paramat
Copy link
Contributor

@paramat paramat commented on 8f77857 Jul 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure i can be bothered to move them again, i'll try to get used to it first :]

Please sign in to comment.