assert improvements (#72)

- Improvement: Asserts gives an immediate ERROR log entry with callstack and reason
- Improvement: Asserts flushes the log before sending error report to Sentry

5 files changed, 83 insertions, 38 deletions

diff --git a/src/zencore/include/zencore/logging.h b/src/zencore/include/zencore/logging.h
index 6d44e31df..bffbe84b4 100644
--- a/src/zencore/include/zencore/logging.h
+++ b/src/zencore/include/zencore/logging.h
@@ -26,6 +26,7 @@ namespace zen::logging {
 void InitializeLogging();
 void ShutdownLogging();
 bool EnableVTMode();
+void FlushLogging();
 
 LoggerRef Default();
 void	  SetDefault(std::string_view NewDefaultLoggerId);
diff --git a/src/zencore/include/zencore/zencore.h b/src/zencore/include/zencore/zencore.h
index cd1a34c7b..d21c0e7e2 100644
--- a/src/zencore/include/zencore/zencore.h
+++ b/src/zencore/include/zencore/zencore.h
@@ -47,40 +47,25 @@ public:
 	struct CallstackFrames* _Callstack = nullptr;
 };
 
+struct CallstackFrames;
+
 struct AssertImpl
 {
-	ZEN_FORCENOINLINE ZEN_DEBUG_SECTION AssertImpl() : PrevAssertImpl(CurrentAssertImpl) { CurrentAssertImpl = this; }
-	virtual ZEN_FORCENOINLINE			ZEN_DEBUG_SECTION ~AssertImpl() { CurrentAssertImpl = PrevAssertImpl; }
+	ZEN_FORCENOINLINE ZEN_DEBUG_SECTION AssertImpl();
+	virtual ZEN_FORCENOINLINE			ZEN_DEBUG_SECTION ~AssertImpl();
 
 	static void ZEN_FORCENOINLINE ZEN_DEBUG_SECTION ExecAssert
-		[[noreturn]] (const char* Filename, int LineNumber, const char* FunctionName, const char* Msg)
-	{
-		AssertImpl* AssertImpl = CurrentAssertImpl;
-		while (AssertImpl)
-		{
-			AssertImpl->OnAssert(Filename, LineNumber, FunctionName, Msg);
-			AssertImpl = AssertImpl->PrevAssertImpl;
-		}
-		ThrowAssertException(Filename, LineNumber, FunctionName, Msg);
-	}
-
-	virtual void ZEN_FORCENOINLINE ZEN_DEBUG_SECTION OnAssert(const char* Filename,
-															  int		  LineNumber,
-															  const char* FunctionName,
-															  const char* Msg)
-	{
-		ZEN_UNUSED(Filename);
-		ZEN_UNUSED(LineNumber);
-		ZEN_UNUSED(FunctionName);
-		ZEN_UNUSED(Msg);
-	}
+		[[noreturn]] (const char* Filename, int LineNumber, const char* FunctionName, const char* Msg);
+
+	virtual void ZEN_FORCENOINLINE ZEN_DEBUG_SECTION
+	OnAssert(const char* Filename, int LineNumber, const char* FunctionName, const char* Msg, CallstackFrames* Callstack);
 
 protected:
 	static void ZEN_FORCENOINLINE ZEN_DEBUG_SECTION ThrowAssertException
-		[[noreturn]] (const char* Filename, int LineNumber, const char* FunctionName, const char* Msg);
+		[[noreturn]] (const char* Filename, int LineNumber, const char* FunctionName, const char* Msg, CallstackFrames* Callstack);
 	static AssertImpl* CurrentAssertImpl;
 	static AssertImpl  DefaultAssertImpl;
-	AssertImpl*		   PrevAssertImpl = nullptr;
+	AssertImpl*		   NextAssertImpl = nullptr;
 };
 
 }  // namespace zen
diff --git a/src/zencore/logging.cpp b/src/zencore/logging.cpp
index 90f4e2428..1a0a91b3d 100644
--- a/src/zencore/logging.cpp
+++ b/src/zencore/logging.cpp
@@ -392,6 +392,12 @@ EnableVTMode()
 	return true;
 }
 
+void
+FlushLogging()
+{
+	spdlog::details::registry::instance().flush_all();
+}
+
 }  // namespace zen::logging
 
 namespace zen {
diff --git a/src/zencore/zencore.cpp b/src/zencore/zencore.cpp
index c4fcc89de..9c48b355b 100644
--- a/src/zencore/zencore.cpp
+++ b/src/zencore/zencore.cpp
@@ -87,8 +87,64 @@ AssertException::FullDescription() const noexcept
 	return what();
 }
 
+AssertImpl::AssertImpl() : NextAssertImpl(nullptr)
+{
+	AssertImpl** WriteAssertPtr = &CurrentAssertImpl;
+	while (*WriteAssertPtr)
+	{
+		WriteAssertPtr = &(*WriteAssertPtr)->NextAssertImpl;
+	}
+	*WriteAssertPtr = this;
+}
+
+AssertImpl::~AssertImpl()
+{
+	AssertImpl** WriteAssertPtr = &CurrentAssertImpl;
+	while ((*WriteAssertPtr) != this)
+	{
+		ZEN_ASSERT((*WriteAssertPtr) != nullptr);
+		WriteAssertPtr = &(*WriteAssertPtr)->NextAssertImpl;
+	}
+	*WriteAssertPtr = NextAssertImpl;
+}
+
 void
-AssertImpl::ThrowAssertException(const char* Filename, int LineNumber, const char* FunctionName, const char* Msg)
+AssertImpl::ExecAssert(const char* Filename, int LineNumber, const char* FunctionName, const char* Msg)
+{
+	void*	 Frames[8];
+	uint32_t FrameCount = GetCallstack(2, 8, Frames);
+
+	CallstackFrames* Callstack = CreateCallstack(FrameCount, Frames);
+
+	AssertImpl* AssertImpl = CurrentAssertImpl;
+	while (AssertImpl)
+	{
+		AssertImpl->OnAssert(Filename, LineNumber, FunctionName, Msg, Callstack);
+		AssertImpl = AssertImpl->NextAssertImpl;
+	}
+	ThrowAssertException(Filename, LineNumber, FunctionName, Msg, Callstack);
+}
+void
+AssertImpl::OnAssert(const char* Filename, int LineNumber, const char* FunctionName, const char* Msg, CallstackFrames* Callstack)
+{
+	ZEN_UNUSED(FunctionName);
+
+	fmt::basic_memory_buffer<char, 2048> Message;
+	auto								 Appender = fmt::appender(Message);
+	fmt::format_to(Appender, "{}({}): ZEN_ASSERT({})\n{}", Filename, LineNumber, Msg, CallstackToString(Callstack));
+	Message.push_back('\0');
+
+	// We use direct ZEN_LOG here instead of ZEN_ERROR as we don't care about *this* code location in the log
+	ZEN_LOG(Log(), zen::logging::level::Err, "{}", Message.data());
+	zen::logging::FlushLogging();
+}
+
+void
+AssertImpl::ThrowAssertException(const char*	  Filename,
+								 int			  LineNumber,
+								 const char*	  FunctionName,
+								 const char*	  Msg,
+								 CallstackFrames* Callstack)
 {
 	ZEN_UNUSED(FunctionName);
 	fmt::basic_memory_buffer<char, 2048> Message;
@@ -96,9 +152,7 @@ AssertImpl::ThrowAssertException(const char* Filename, int LineNumber, const cha
 	fmt::format_to(Appender, "{}({}): {}", Filename, LineNumber, Msg);
 	Message.push_back('\0');
 
-	void*	 Frames[8];
-	uint32_t FrameCount = GetCallstack(3, 8, Frames);
-	throw AssertException(Message.data(), CreateCallstack(FrameCount, Frames));
+	throw AssertException(Message.data(), Callstack);
 }
 
 void refcount_forcelink();
@@ -239,11 +293,10 @@ TEST_CASE("Assert.Custom")
 {
 	struct MyAssertImpl : AssertImpl
 	{
-		virtual void ZEN_FORCENOINLINE ZEN_DEBUG_SECTION OnAssert(const char* Filename,
-																  int		  LineNumber,
-																  const char* FunctionName,
-																  const char* Msg)
+		virtual void ZEN_FORCENOINLINE ZEN_DEBUG_SECTION
+		OnAssert(const char* Filename, int LineNumber, const char* FunctionName, const char* Msg, CallstackFrames* Callstack)
 		{
+			ZEN_UNUSED(Callstack);
 			AssertFileName = Filename;
 			Line		   = LineNumber;
 			FuncName	   = FunctionName;
diff --git a/src/zenserver/sentryintegration.cpp b/src/zenserver/sentryintegration.cpp
index a8d967985..2f68a269b 100644
--- a/src/zenserver/sentryintegration.cpp
+++ b/src/zenserver/sentryintegration.cpp
@@ -31,10 +31,8 @@ namespace sentry {
 
 struct SentryAssertImpl : zen::AssertImpl
 {
-	virtual void ZEN_FORCENOINLINE ZEN_DEBUG_SECTION OnAssert(const char* Filename,
-															  int		  LineNumber,
-															  const char* FunctionName,
-															  const char* Msg) override;
+	virtual void ZEN_FORCENOINLINE ZEN_DEBUG_SECTION
+	OnAssert(const char* Filename, int LineNumber, const char* FunctionName, const char* Msg, zen::CallstackFrames* Callstack) override;
 };
 
 class sentry_sink final : public spdlog::sinks::base_sink<spdlog::details::null_mutex>
@@ -103,8 +101,10 @@ sentry_sink::flush_()
 }
 
 void
-SentryAssertImpl::OnAssert(const char* Filename, int LineNumber, const char* FunctionName, const char* Msg)
+SentryAssertImpl::OnAssert(const char* Filename, int LineNumber, const char* FunctionName, const char* Msg, zen::CallstackFrames* Callstack)
 {
+	// Sentry will provide its own callstack
+	ZEN_UNUSED(Callstack);
 	try
 	{
 		std::string	   Message = fmt::format("ASSERT {}:({}) [{}]\n\"{}\"", Filename, LineNumber, FunctionName, Msg);