cmake+ci: run clang-tidy (#512)

* cmake+ci: run clang-tidy

* Remove DESCRIPTION from LEGO1/LegoOmni.mingw.def

* Add initial .clang-tidy and fixes

* fix file perms

* Comment out DESCRIPTION

* Remove LegoEntity::~LegoEntity and MxPresenter::~MxPresenter from mingw's LEGO1.def

* Looks like clang is allergic to the libs in the directx5 SDK

* Update .clang-tidy

* Fix typo in .clang-tidy

* Attempt to generate an action error

* Revert "Attempt to generate an action error"

This reverts commit 96c4c65fed.

* cmake: test with -Wparentheses + optionally with -Werror

* ci: -k0 is a Ninja argument

* Use -Werror only for msys2 builds

* cmake: only emit warnings for specific warnings

* cmake: and don't do -Werror/-WX anymore

* Fix warnings

* Fix mingw warnings

---------

Co-authored-by: Christian Semmler <mail@csemmler.com>

LEGO1/omni/src/common/mxatomid.cpp

@@ -5,11 +5,13 @@
 // FUNCTION: LEGO1 0x100acf90
 MxAtomId::MxAtomId(const char* p_str, LookupMode p_mode)
 {
-	if (!MxOmni::GetInstance())
+	if (!MxOmni::GetInstance()) {
 		return;
+	}
 
-	if (!AtomIdCounterSet())
+	if (!AtomIdCounterSet()) {
 		return;
+	}
 
 	MxAtomIdCounter* counter = GetCounter(p_str, p_mode);
 	m_internal = counter->GetKey()->GetData();
@@ -25,14 +27,17 @@ MxAtomId::~MxAtomId()
 // FUNCTION: LEGO1 0x100acfe0
 void MxAtomId::Destroy()
 {
-	if (!m_internal)
+	if (!m_internal) {
 		return;
+	}
 
-	if (!MxOmni::GetInstance())
+	if (!MxOmni::GetInstance()) {
 		return;
+	}
 
-	if (!AtomIdCounterSet())
+	if (!AtomIdCounterSet()) {
 		return;
+	}
 
 #ifdef COMPAT_MODE
 	MxAtomIdCounterSet::iterator it;
@@ -51,8 +56,9 @@ void MxAtomId::Destroy()
 // FUNCTION: LEGO1 0x100ad1c0
 MxAtomId& MxAtomId::operator=(const MxAtomId& p_atomId)
 {
-	if (m_internal)
+	if (m_internal) {
 		Destroy();
+	}
 
 	if (p_atomId.m_internal && MxOmni::GetInstance() && AtomIdCounterSet()) {
 		MxAtomIdCounter* counter = GetCounter(p_atomId.m_internal, e_exact);

LEGO1/omni/src/common/mxatomidcounter.cpp

@@ -14,6 +14,7 @@ void MxAtomIdCounter::Inc()
 // FUNCTION: LEGO1 0x100ad800
 void MxAtomIdCounter::Dec()
 {
-	if (m_value)
+	if (m_value) {
 		m_value--;
+	}
 }

LEGO1/omni/src/common/mxcompositepresenter.cpp

@@ -50,16 +50,18 @@ MxResult MxCompositePresenter::StartAction(MxStreamController* p_controller, MxD
 
 			if (presenter && presenter->AddToManager() == SUCCESS) {
 				presenter->SetCompositePresenter(this);
-				if (presenter->StartAction(p_controller, action) == SUCCESS)
+				if (presenter->StartAction(p_controller, action) == SUCCESS) {
 					success = TRUE;
+				}
 			}
 
 			if (success) {
 				action->SetOrigin(this);
 				m_list.push_back(presenter);
 			}
-			else if (presenter)
+			else if (presenter) {
 				delete presenter;
+			}
 		}
 
 		result = SUCCESS;
@@ -73,8 +75,9 @@ void MxCompositePresenter::EndAction()
 {
 	MxAutoLocker lock(&m_criticalSection);
 
-	if (!m_action)
+	if (!m_action) {
 		return;
+	}
 
 	((MxDSMultiAction*) m_action)->GetActionList()->DeleteAll(FALSE);
 
@@ -139,15 +142,18 @@ void MxCompositePresenter::VTable0x58(MxEndActionNotificationParam& p_param)
 		MxDSActionList* actions = ((MxDSMultiAction*) m_action)->GetActionList();
 		MxDSActionListCursor cursor(actions);
 
-		if (cursor.Find(action))
+		if (cursor.Find(action)) {
 			cursor.Detach();
+		}
 	}
 
-	if (presenter)
+	if (presenter) {
 		delete presenter;
+	}
 
-	if (action)
+	if (action) {
 		delete action;
+	}
 
 	if (m_list.empty()) {
 		EndAction();
@@ -155,8 +161,9 @@ void MxCompositePresenter::VTable0x58(MxEndActionNotificationParam& p_param)
 	else {
 		if (m_action->IsA("MxDSSerialAction") && it != m_list.end()) {
 			MxPresenter* presenter = *it;
-			if (presenter->GetCurrentTickleState() == e_idle)
+			if (presenter->GetCurrentTickleState() == e_idle) {
 				presenter->SetTickleState(e_ready);
+			}
 		}
 	}
 }
@@ -171,14 +178,16 @@ void MxCompositePresenter::VTable0x5c(MxNotificationParam& p_param)
 			if (*it == presenter) {
 				m_list.erase(it++);
 
-				if (presenter->GetCurrentTickleState() == e_idle)
+				if (presenter->GetCurrentTickleState() == e_idle) {
 					presenter->SetTickleState(e_ready);
+				}
 
 				MxDSActionList* actions = ((MxDSMultiAction*) m_action)->GetActionList();
 				MxDSActionListCursor cursor(actions);
 
-				if (cursor.Find(presenter->GetAction()))
+				if (cursor.Find(presenter->GetAction())) {
 					cursor.Detach();
+				}
 
 				if (m_list.empty()) {
 					EndAction();
@@ -186,8 +195,9 @@ void MxCompositePresenter::VTable0x5c(MxNotificationParam& p_param)
 				else {
 					if (m_action->IsA("MxDSSerialAction")) {
 						MxPresenter* presenter = *it;
-						if (presenter->GetCurrentTickleState() == e_idle)
+						if (presenter->GetCurrentTickleState() == e_idle) {
 							presenter->SetTickleState(e_ready);
+						}
 					}
 				}
 
@@ -205,13 +215,15 @@ void MxCompositePresenter::VTable0x60(MxPresenter* p_presenter)
 	for (MxCompositePresenterList::iterator it = m_list.begin(); it != m_list.end(); it++) {
 		if (*it == p_presenter) {
 			if (++it == m_list.end()) {
-				if (m_compositePresenter)
+				if (m_compositePresenter) {
 					m_compositePresenter->VTable0x60(this);
+				}
 			}
 			else if (m_action->IsA("MxDSSerialAction")) {
 				MxPresenter* presenter = *it;
-				if (presenter->GetCurrentTickleState() == e_idle)
+				if (presenter->GetCurrentTickleState() == e_idle) {
 					presenter->SetTickleState(e_ready);
+				}
 			}
 			return;
 		}
@@ -227,8 +239,9 @@ void MxCompositePresenter::SetTickleState(TickleState p_tickleState)
 		MxPresenter* presenter = *it;
 		presenter->SetTickleState(p_tickleState);
 
-		if (m_action->IsA("MxDSSerialAction") && p_tickleState == e_ready)
+		if (m_action->IsA("MxDSSerialAction") && p_tickleState == e_ready) {
 			return;
+		}
 	}
 }
 
@@ -248,8 +261,9 @@ MxBool MxCompositePresenter::HasTickleStatePassed(TickleState p_tickleState)
 {
 	for (MxCompositePresenterList::iterator it = m_list.begin(); it != m_list.end(); it++) {
 		MxPresenter* presenter = *it;
-		if (!presenter->HasTickleStatePassed(p_tickleState))
+		if (!presenter->HasTickleStatePassed(p_tickleState)) {
 			return FALSE;
+		}
 	}
 
 	return TRUE;

LEGO1/omni/src/common/mxmediamanager.cpp

@@ -50,8 +50,9 @@ void MxMediaManager::Destroy()
 {
 	MxAutoLocker lock(&this->m_criticalSection);
 
-	if (this->m_presenters)
+	if (this->m_presenters) {
 		delete this->m_presenters;
+	}
 
 	Init();
 }
@@ -63,13 +64,15 @@ MxResult MxMediaManager::Tickle()
 	MxPresenter* presenter;
 	MxPresenterListCursor cursor(this->m_presenters);
 
-	while (cursor.Next(presenter))
+	while (cursor.Next(presenter)) {
 		presenter->Tickle();
+	}
 
 	cursor.Reset();
 
-	while (cursor.Next(presenter))
+	while (cursor.Next(presenter)) {
 		presenter->PutData();
+	}
 
 	return SUCCESS;
 }
@@ -88,8 +91,9 @@ void MxMediaManager::UnregisterPresenter(MxPresenter& p_presenter)
 	MxAutoLocker lock(&this->m_criticalSection);
 	MxPresenterListCursor cursor(this->m_presenters);
 
-	if (cursor.Find(&p_presenter))
+	if (cursor.Find(&p_presenter)) {
 		cursor.Detach();
+	}
 }
 
 // FUNCTION: LEGO1 0x100b8ac0
@@ -99,6 +103,7 @@ void MxMediaManager::StopPresenters()
 	MxPresenter* presenter;
 	MxPresenterListCursor cursor(this->m_presenters);
 
-	while (cursor.Next(presenter))
+	while (cursor.Next(presenter)) {
 		presenter->EndAction();
+	}
 }

LEGO1/omni/src/common/mxmediapresenter.cpp

@@ -26,21 +26,25 @@ void MxMediaPresenter::Destroy(MxBool p_fromDestructor)
 	{
 		MxAutoLocker lock(&m_criticalSection);
 
-		if (m_currentChunk && m_subscriber)
+		if (m_currentChunk && m_subscriber) {
 			m_subscriber->DestroyChunk(m_currentChunk);
+		}
 
-		if (m_subscriber)
+		if (m_subscriber) {
 			delete m_subscriber;
+		}
 
-		if (m_loopingChunkCursor)
+		if (m_loopingChunkCursor) {
 			delete m_loopingChunkCursor;
+		}
 
 		if (m_loopingChunks) {
 			MxStreamChunkListCursor cursor(m_loopingChunks);
 			MxStreamChunk* chunk;
 
-			while (cursor.Next(chunk))
+			while (cursor.Next(chunk)) {
 				chunk->Release();
+			}
 
 			delete m_loopingChunks;
 		}
@@ -48,8 +52,9 @@ void MxMediaPresenter::Destroy(MxBool p_fromDestructor)
 		Init();
 	}
 
-	if (!p_fromDestructor)
+	if (!p_fromDestructor) {
 		MxPresenter::Destroy();
+	}
 }
 
 // FUNCTION: LEGO1 0x100b5650
@@ -102,16 +107,18 @@ MxResult MxMediaPresenter::StartAction(MxStreamController* p_controller, MxDSAct
 			m_loopingChunks = new MxStreamChunkList;
 			m_loopingChunkCursor = new MxStreamChunkListCursor(m_loopingChunks);
 
-			if (!m_loopingChunks && !m_loopingChunkCursor)
+			if (!m_loopingChunks && !m_loopingChunkCursor) {
 				goto done;
+			}
 		}
 
 		if (p_controller) {
 			m_subscriber = new MxDSSubscriber;
 
 			if (!m_subscriber ||
-				m_subscriber->Create(p_controller, p_action->GetObjectId(), p_action->GetUnknown24()) != SUCCESS)
+				m_subscriber->Create(p_controller, p_action->GetObjectId(), p_action->GetUnknown24()) != SUCCESS) {
 				goto done;
+			}
 		}
 
 		result = SUCCESS;
@@ -126,8 +133,9 @@ void MxMediaPresenter::EndAction()
 {
 	MxAutoLocker lock(&m_criticalSection);
 
-	if (!m_action)
+	if (!m_action) {
 		return;
+	}
 
 	m_currentChunk = NULL;
 
@@ -198,18 +206,22 @@ void MxMediaPresenter::StreamingTickle()
 void MxMediaPresenter::RepeatingTickle()
 {
 	if (IsEnabled() && !m_currentChunk) {
-		if (m_loopingChunkCursor)
-			if (!m_loopingChunkCursor->Next(m_currentChunk))
+		if (m_loopingChunkCursor) {
+			if (!m_loopingChunkCursor->Next(m_currentChunk)) {
 				m_loopingChunkCursor->Next(m_currentChunk);
+			}
+		}
 
 		if (m_currentChunk) {
 			MxLong time = m_currentChunk->GetTime();
-			if (time <= m_action->GetElapsedTime() % m_action->GetLoopCount())
+			if (time <= m_action->GetElapsedTime() % m_action->GetLoopCount()) {
 				ProgressTickleState(e_unk5);
+			}
 		}
 		else {
-			if (m_action->GetElapsedTime() >= m_action->GetStartTime() + m_action->GetDuration())
+			if (m_action->GetElapsedTime() >= m_action->GetStartTime() + m_action->GetDuration()) {
 				ProgressTickleState(e_unk5);
+			}
 		}
 	}
 }
@@ -248,8 +260,9 @@ void MxMediaPresenter::Enable(MxBool p_enable)
 			SetTickleState(e_repeating);
 		}
 		else {
-			if (m_loopingChunkCursor)
+			if (m_loopingChunkCursor) {
 				m_loopingChunkCursor->Reset();
+			}
 			m_currentChunk = NULL;
 			SetTickleState(e_done);
 		}

LEGO1/omni/src/common/mxpresenter.cpp

@@ -49,8 +49,9 @@ MxResult MxPresenter::StartAction(MxStreamController*, MxDSAction* p_action)
 // FUNCTION: LEGO1 0x100b4e40
 void MxPresenter::EndAction()
 {
-	if (this->m_action == NULL)
+	if (this->m_action == NULL) {
 		return;
+	}
 
 	MxAutoLocker lock(&this->m_criticalSection);
 
@@ -98,8 +99,9 @@ void MxPresenter::ParseExtra()
 
 			m_action->SetFlags(m_action->GetFlags() | MxDSAction::c_world);
 
-			if (result)
+			if (result) {
 				SendToCompositePresenter(MxOmni::GetInstance());
+			}
 		}
 	}
 }
@@ -133,28 +135,33 @@ MxResult MxPresenter::Tickle()
 	case e_ready:
 		this->ReadyTickle();
 
-		if (m_currentTickleState != e_starting)
+		if (m_currentTickleState != e_starting) {
 			break;
+		}
 	case e_starting:
 		this->StartingTickle();
 
-		if (m_currentTickleState != e_streaming)
+		if (m_currentTickleState != e_streaming) {
 			break;
+		}
 	case e_streaming:
 		this->StreamingTickle();
 
-		if (m_currentTickleState != e_repeating)
+		if (m_currentTickleState != e_repeating) {
 			break;
+		}
 	case e_repeating:
 		this->RepeatingTickle();
 
-		if (m_currentTickleState != e_unk5)
+		if (m_currentTickleState != e_unk5) {
 			break;
+		}
 	case e_unk5:
 		this->Unk5Tickle();
 
-		if (m_currentTickleState != e_done)
+		if (m_currentTickleState != e_done) {
 			break;
+		}
 	case e_done:
 		this->DoneTickle();
 	default:
@@ -170,10 +177,12 @@ void MxPresenter::Enable(MxBool p_enable)
 	if (this->m_action && this->IsEnabled() != p_enable) {
 		MxU32 flags = this->m_action->GetFlags();
 
-		if (p_enable)
+		if (p_enable) {
 			this->m_action->SetFlags(flags | MxDSAction::c_enabled);
-		else
+		}
+		else {
 			this->m_action->SetFlags(flags & ~MxDSAction::c_enabled);
+		}
 	}
 }
 

LEGO1/omni/src/common/mxticklemanager.cpp

@@ -45,8 +45,9 @@ MxResult MxTickleManager::Tickle()
 		else {
 			it++;
 
-			if (client->GetLastUpdateTime() > time)
+			if (client->GetLastUpdateTime() > time) {
 				client->SetLastUpdateTime(-client->GetTickleInterval());
+			}
 
 			if ((client->GetTickleInterval() + client->GetLastUpdateTime()) < time) {
 				client->GetClient()->Tickle();
@@ -64,8 +65,9 @@ void MxTickleManager::RegisterClient(MxCore* p_client, MxTime p_interval)
 	MxTime interval = GetClientTickleInterval(p_client);
 	if (interval == TICKLE_MANAGER_NOT_FOUND) {
 		MxTickleClient* client = new MxTickleClient(p_client, p_interval);
-		if (client != NULL)
+		if (client != NULL) {
 			m_clients.push_back(client);
+		}
 	}
 }
 
@@ -102,8 +104,9 @@ MxTime MxTickleManager::GetClientTickleInterval(MxCore* p_client)
 	MxTickleClientPtrList::iterator it = m_clients.begin();
 	while (it != m_clients.end()) {
 		MxTickleClient* client = *it;
-		if ((client->GetClient() == p_client) && ((client->GetFlags() & TICKLE_MANAGER_FLAG_DESTROY) == 0))
+		if ((client->GetClient() == p_client) && ((client->GetFlags() & TICKLE_MANAGER_FLAG_DESTROY) == 0)) {
 			return client->GetTickleInterval();
+		}
 
 		it++;
 	}

LEGO1/omni/src/common/mxutil.cpp

@@ -35,15 +35,17 @@ MxBool GetRectIntersection(
 	MxRect32 rect(0, 0, *p_width, *p_height);
 	rect.AddPoint(rect1Origin);
 
-	if (!rect.IntersectsWith(rect1))
+	if (!rect.IntersectsWith(rect1)) {
 		return FALSE;
+	}
 
 	rect.Intersect(rect1);
 	rect.SubtractPoint(rect1Origin);
 	rect.AddPoint(rect2Origin);
 
-	if (!rect.IntersectsWith(rect2))
+	if (!rect.IntersectsWith(rect2)) {
 		return FALSE;
+	}
 
 	rect.Intersect(rect2);
 	rect.SubtractPoint(rect2Origin);
@@ -93,8 +95,9 @@ MxBool KeyValueStringParse(char* p_outputValue, const char* p_key, const char* p
 				char* cur = &token[strlen(p_key)];
 				cur++;
 				while (*cur != ',') {
-					if (*cur == ' ' || *cur == '\0' || *cur == '\t' || *cur == '\n' || *cur == '\r')
+					if (*cur == ' ' || *cur == '\0' || *cur == '\t' || *cur == '\n' || *cur == '\r') {
 						break;
+					}
 					*p_outputValue++ = *cur++;
 				}
 				*p_outputValue = '\0';
@@ -115,8 +118,8 @@ MxBool KeyValueStringParse(char* p_outputValue, const char* p_key, const char* p
 MxBool ContainsPresenter(MxCompositePresenterList& p_presenterList, MxPresenter* p_presenter)
 {
 	for (MxCompositePresenterList::iterator it = p_presenterList.begin(); it != p_presenterList.end(); it++) {
-		if (p_presenter == *it || (*it)->IsA("MxCompositePresenter") &&
-									  ContainsPresenter(((MxCompositePresenter*) *it)->GetList(), p_presenter)) {
+		if (p_presenter == *it || ((*it)->IsA("MxCompositePresenter") &&
+								   ContainsPresenter(((MxCompositePresenter*) *it)->GetList(), p_presenter))) {
 			return TRUE;
 		}
 	}

LEGO1/omni/src/common/mxvariabletable.cpp

@@ -41,8 +41,9 @@ void MxVariableTable::SetVariable(MxVariable* p_var)
 	MxHashTableCursor<MxVariable*> cursor(this);
 	MxBool found = cursor.Find(p_var);
 
-	if (found)
+	if (found) {
 		cursor.DeleteMatch();
+	}
 
 	MxHashTable<MxVariable*>::Add(p_var);
 }