Refactoring random stuff

[crodnu]

What it says in the tin, refactorng random crap to learn the engine internals while doing something marginally useful.

[hhyyrylainen]

This broke some tests (I didn't review the code yet) : at least the quaternion math is broken.

[hhyyrylainen]

Doesn't compile on linux (circleci build fails):

[ 44%] Building CXX object Engine/CMakeFiles/Engine.dir/Common/StartEndIndex.cpp.o
/root/leviathan/Engine/Common/StartEndIndex.cpp: In function 'std::ostream& operator<<(std::ostream&, const Leviathan::StartEndIndex&)':
/root/leviathan/Engine/Common/StartEndIndex.cpp:8:12: error: no match for 'operator<<' (operand types are 'std::ostream' {aka 'std::basic_ostream<char>'} and 'const char [2]')
     stream << "[";
     ~~~~~~~^~~~~~
/root/leviathan/Engine/Common/StartEndIndex.cpp:6:25: note: candidate: 'std::ostream& operator<<(std::ostream&, const Leviathan::StartEndIndex&)' <near match>
 DLLEXPORT std::ostream& operator<<(std::ostream& stream, const StartEndIndex& value)
                         ^~~~~~~~
/root/leviathan/Engine/Common/StartEndIndex.cpp:6:25: note:   conversion of argument 2 would be ill-formed:
/root/leviathan/Engine/Common/StartEndIndex.cpp:8:15: error: invalid user-defined conversion from 'const char [2]' to 'const Leviathan::StartEndIndex&' [-fpermissive]
     stream << "[";
               ^~~
In file included from /root/leviathan/Engine/Common/StartEndIndex.cpp:1:
/root/leviathan/Engine/Common/StartEndIndex.h:34:11: note: candidate is: 'constexpr Leviathan::StartEndIndex::StartEndIndex(size_t)' <near match>
 constexpr StartEndIndex::StartEndIndex(size_t start) noexcept : Start(start) {}
           ^~~~~~~~~~~~~
/root/leviathan/Engine/Common/StartEndIndex.h:34:11: note:   conversion of argument 1 would be ill-formed:
/root/leviathan/Engine/Common/StartEndIndex.cpp:8:15: error: invalid conversion from 'const char*' to 'size_t' {aka 'long unsigned int'} [-fpermissive]
     stream << "[";
               ^~~
/root/leviathan/Engine/Common/StartEndIndex.cpp:8:15: error: invalid conversion from 'const char*' to 'size_t' {aka 'long unsigned int'} [-fpermissive]
In file included from /root/leviathan/Engine/Common/StartEndIndex.cpp:1:
/root/leviathan/Engine/Common/StartEndIndex.h:34:47: note:   initializing argument 1 of 'constexpr Leviathan::StartEndIndex::StartEndIndex(size_t)'
 constexpr StartEndIndex::StartEndIndex(size_t start) noexcept : Start(start) {}
                                        ~~~~~~~^~~~~
/root/leviathan/Engine/Common/StartEndIndex.h:27:25: note: candidate: 'std::ostream& Leviathan::operator<<(std::ostream&, const Leviathan::StartEndIndex&)' <near match>
 DLLEXPORT std::ostream& operator<<(
                         ^~~~~~~~
/root/leviathan/Engine/Common/StartEndIndex.h:27:25: note:   conversion of argument 2 would be ill-formed:
/root/leviathan/Engine/Common/StartEndIndex.cpp:8:15: error: invalid user-defined conversion from 'const char [2]' to 'const Leviathan::StartEndIndex&' [-fpermissive]
     stream << "[";
               ^~~
In file included from /root/leviathan/Engine/Common/StartEndIndex.cpp:1:
/root/leviathan/Engine/Common/StartEndIndex.h:34:11: note: candidate is: 'constexpr Leviathan::StartEndIndex::StartEndIndex(size_t)' <near match>
 constexpr StartEndIndex::StartEndIndex(size_t start) noexcept : Start(start) {}
           ^~~~~~~~~~~~~
/root/leviathan/Engine/Common/StartEndIndex.h:34:11: note:   conversion of argument 1 would be ill-formed:
/root/leviathan/Engine/Common/StartEndIndex.cpp:8:15: error: invalid conversion from 'const char*' to 'size_t' {aka 'long unsigned int'} [-fpermissive]
     stream << "[";
               ^~~
/root/leviathan/Engine/Common/StartEndIndex.cpp:8:15: error: invalid conversion from 'const char*' to 'size_t' {aka 'long unsigned int'} [-fpermissive]
In file included from /root/leviathan/Engine/Common/StartEndIndex.cpp:1:
/root/leviathan/Engine/Common/StartEndIndex.h:34:47: note:   initializing argument 1 of 'constexpr Leviathan::StartEndIndex::StartEndIndex(size_t)'
 constexpr StartEndIndex::StartEndIndex(size_t start) noexcept : Start(start) {}
                                        ~~~~~~~^~~~~
In file included from /usr/include/c++/8/bits/basic_string.h:48,
                 from /usr/include/c++/8/string:52,
                 from /root/leviathan/Engine/Define.h:16,
                 from /root/leviathan/Engine/Common/StartEndIndex.h:3,
                 from /root/leviathan/Engine/Common/StartEndIndex.cpp:1:
/usr/include/c++/8/string_view:545:5: note: candidate: 'template<class _CharT, class _Traits> std::basic_ostream<_CharT, _Traits>& std::operator<<(std::basic_ostream<_CharT, _Traits>&, std::basic_string_view<_CharT, _Traits>)'
     operator<<(basic_ostream<_CharT, _Traits>& __os,
     ^~~~~~~~
/usr/include/c++/8/string_view:545:5: note:   template argument deduction/substitution failed:
/root/leviathan/Engine/Common/StartEndIndex.cpp:8:15: note:   mismatched types 'std::basic_string_view<_CharT, _Traits>' and 'const char*'
     stream << "[";
               ^~~
In file included from /usr/include/c++/8/string:52,
                 from /root/leviathan/Engine/Define.h:16,
                 from /root/leviathan/Engine/Common/StartEndIndex.h:3,
                 from /root/leviathan/Engine/Common/StartEndIndex.cpp:1:
/usr/include/c++/8/bits/basic_string.h:6314:5: note: candidate: 'template<class _CharT, class _Traits, class _Alloc> std::basic_ostream<_CharT, _Traits>& std::operator<<(std::basic_ostream<_CharT, _Traits>&, const std::__cxx11::basic_string<_CharT, _Traits, _Allocator>&)'
     operator<<(basic_ostream<_CharT, _Traits>& __os,
     ^~~~~~~~
/usr/include/c++/8/bits/basic_string.h:6314:5: note:   template argument deduction/substitution failed:
/root/leviathan/Engine/Common/StartEndIndex.cpp:8:15: note:   mismatched types 'const std::__cxx11::basic_string<_CharT, _Traits, _Allocator>' and 'const char [2]'
     stream << "[";
               ^~~
/root/leviathan/Engine/Common/StartEndIndex.cpp:10:16: error: ambiguous overload for 'operator<<' (operand types are 'std::ostream' {aka 'std::basic_ostream<char>'} and 'const long unsigned int')
         stream << value.Start.value();
         ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~

(and so on, I didn't copy paste everything)

Also there is a warning (which I assume means you haven't ran clang format on the files you have edited):

/root/leviathan/Engine/Utility/ComplainOnce.cpp: In member function 'bool Leviathan::ComplainOnce::ThreadUnsafeComplainOnce::wasErrorLogged(const string&)':
/root/leviathan/Engine/Utility/ComplainOnce.cpp:37:5: warning: this 'for' clause does not guard... [-Wmisleading-indentation]
     for(auto iter = FiredErrors.begin(); iter != FiredErrors.end(); ++iter)
     ^~~
/root/leviathan/Engine/Utility/ComplainOnce.cpp:40:2: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'for'
  return false;
  ^~~~~~

[crodnu]

\o/

[hhyyrylainen]

I have now reviewed the actual code and first of there are some good changes that I would merge if they were in a separate commit (so I could cherry-pick it): these are the removal of the maths files that never got written. Then there are some good ideas like the usage of std::optional and making warn once thread safe, unfortunately these are implemented in a way that I don't like them. There should be no calls to .has_value() and the StringIterator debugging is most likely broken due to the change. And the extra class of having a Monitor is not good. Everything that is thread safe should inherit from ThreadSafe and use GUARD_LOCK();. Then there are the vector types, which I think have been split into too many files and even with splitting Types.h into 3 files is in my opinion quite unnecessary. Also the fact that the implementations of the functions for these classes are outside the class is not consistent with the style I want to go for. And I also think that the nan check debugging help is broken here as well.
So in summary I'm not going to accept this pull request as is.

[hhyyrylainen]

Why duplicate EPSILON here?

[crodnu]

It was a magic number before that i made into a constant, i guess there is another constant for that? No idea if those functions are actually used anywhere to begin with tho, they're definetely not used in the vector code, and if they are they should probably be rewritten to be inlined (and check if the abs call is inlined as well)

[hhyyrylainen]

The math code was written when I was writing a model importer, it was abandoned when I switched to Ogre. But the math code should be kept.

There are a bunch of constants in Define.h including Leviathan::EPSILON