Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/non blocking for loading game info #240

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

pavanvo
Copy link

@pavanvo pavanvo commented Aug 7, 2022

This PR solving issue with main thread blocking, on loading .xml file with saved game.
Also this code can be used somewhere else
fixes #233

if(SystemFlags::VERBOSE_MODE_ENABLED) printf("Before load of XML\n");
versionWarningLabel.setText("");
infoTextLabel.setText("Loading...");
Copy link
Contributor

Choose a reason for hiding this comment

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

Spaces vs tabs, this codebase uses mostly tabs, let's try to keep it consistent :)

@@ -23,10 +23,12 @@
#endif

#include "rapidxml/rapidxml.hpp"
#include "callback.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

you forgot to include memory

Copy link
Contributor

@Rampoina Rampoina left a comment

Choose a reason for hiding this comment

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

Tested, works fine on my system 👍 (after including memory)

@@ -0,0 +1,71 @@
#pragma once
Copy link
Contributor

Choose a reason for hiding this comment

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

This is done in other places of the code with https://github.com/MegaGlest/megaglest-source/blob/develop/source/shared_lib/include/platform/common/simple_threads.h#L71
Although honestly your way seems cleaner to me.

Copy link
Contributor

@andy5995 andy5995 left a comment

Choose a reason for hiding this comment

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

I see this is passing the Linux CI, but the build fails for me on Manjaro, using g++ (GCC) 12.1.1 20220730

[ 45%] Building CXX object source/shared_lib/CMakeFiles/libmegaglest.dir/sources/lua/lua_script.cpp.o
In file included from /home/andy/src/megaglest-source/source/shared_lib/include/util/utf8.h:31,
                 from /home/andy/src/megaglest-source/source/shared_lib/sources/graphics/gl/text_renderer_gl.cpp:21:
/home/andy/src/megaglest-source/source/shared_lib/include/util/utf8/checked.h:268:34: warning: 'template<class _Category, class _Tp, class _Distance, class _Pointer, class _Reference> struct std::iterator' is deprecated [-Wdeprecated-declarations]
  268 |     class iterator : public std::iterator <std::bidirectional_iterator_tag, uint32_t> {
      |                                  ^~~~~~~~
In file included from /usr/include/c++/12.1.1/string:45,
                 from /home/andy/src/megaglest-source/source/shared_lib/include/graphics/text_renderer.h:15,
                 from /home/andy/src/megaglest-source/source/shared_lib/include/graphics/gl/text_renderer_gl.h:15,
                 from /home/andy/src/megaglest-source/source/shared_lib/sources/graphics/gl/text_renderer_gl.cpp:12:
/usr/include/c++/12.1.1/bits/stl_iterator_base_types.h:127:34: note: declared here
  127 |     struct _GLIBCXX17_DEPRECATED iterator
      |                                  ^~~~~~~~
In file included from /home/andy/src/megaglest-source/source/shared_lib/include/util/utf8.h:32:
/home/andy/src/megaglest-source/source/shared_lib/include/util/utf8/unchecked.h:179:40: warning: 'template<class _Category, class _Tp, class _Distance, class _Pointer, class _Reference> struct std::iterator' is deprecated [-Wdeprecated-declarations]
  179 |           class iterator : public std::iterator <std::bidirectional_iterator_tag, uint32_t> {
      |                                        ^~~~~~~~
/usr/include/c++/12.1.1/bits/stl_iterator_base_types.h:127:34: note: declared here
  127 |     struct _GLIBCXX17_DEPRECATED iterator
      |                                  ^~~~~~~~
[ 45%] Building CXX object source/shared_lib/CMakeFiles/libmegaglest.dir/sources/map/map_preview.cpp.o
In file included from /home/andy/src/megaglest-source/source/shared_lib/include/lua/lua_script.h:18,
                 from /home/andy/src/megaglest-source/source/shared_lib/sources/lua/lua_script.cpp:12:
/home/andy/src/megaglest-source/source/shared_lib/include/xml/xml_parser.h:156:10: error: 'shared_ptr' in namespace 'std' does not name a template type
  156 |     std::shared_ptr<CallBack<void>> loadAsync(const string &path, const std::map<string,string> &mapTagReplacementValues, bool noValidation=false,bool skipStackCheck=false,bool skipStackTrace=false);
      |          ^~~~~~~~~~
/home/andy/src/megaglest-source/source/shared_lib/include/xml/xml_parser.h:27:1: note: 'std::shared_ptr' is defined in header '<memory>'; did you forget to '#include <memory>'?
   26 | #include "callback.h"
  +++ |+#include <memory>
   27 | #include "data_types.h"
[ 46%] Building CXX object source/shared_lib/CMakeFiles/libmegaglest.dir/sources/platform/common/base_thread.cpp.o
make[2]: *** [source/shared_lib/CMakeFiles/libmegaglest.dir/build.make:706: source/shared_lib/CMakeFiles/libmegaglest.dir/sources/lua/lua_script.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
/home/andy/src/megaglest-source/source/shared_lib/sources/platform/common/base_thread.cpp: In destructor 'virtual Shared::PlatformCommon::BaseThread::~BaseThread()':
/home/andy/src/megaglest-source/source/shared_lib/sources/platform/common/base_thread.cpp:95:17: warning: 'throw' will always call 'terminate' [-Wterminate]
   95 |                 throw megaglest_runtime_error(szBuf);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/andy/src/megaglest-source/source/shared_lib/sources/platform/common/base_thread.cpp:95:17: note: in C++11 destructors default to 'noexcept'
make[1]: *** [CMakeFiles/Makefile2:227: source/shared_lib/CMakeFiles/libmegaglest.dir/all] Error 2
make: *** [Makefile:111: all] Error 2
ERROR: MAKE failed.

@Jammyjamjamman are you able to build this?

@Rampoina
Copy link
Contributor

Rampoina commented Aug 14, 2022

@andy5995 like I mentioned in the other comment, @pavanvo forgot to #include <memory> like the compiler tells you, probably not needed for earlier gcc versions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loading game blocking main thread
3 participants