Feat(plugins): Reload game data without restarting the game #9868

RisingLeaf · 2024-02-25T11:38:47Z

Feature
Related issue: #9137
Related PR: #9068

Summary

You want to quickly disable/enable a plugin but don't like to restart the game every time you switch between plugins?
In that case this PR is for you, this PR allows to reload all game data including images and sounds while in the game.
This will be even more helpful when it will be possible to install plugins in game, I always find it annoying to have to restart the game when I install something new.
As of now it is only possible to reload data when something got changed inside the plugins menu, I could make it so that it is always possible, which I imagine would be helpful to plugin authors who would be able to make changes while the game is running.

Some of the more technical aspects:

Added clear/reset functions to all classes containing data/sounds/sprites
Added a new messenger class to quickly communicate if the game should reload or just shut down
Added a destructor to the sprite class which frees texture memory
There is a small message on the screen during reload to show that the game is doing something and not just a black screen
I tried to touch as little existing code as possible to avoid confusion

I know the visuals could be improved but I see that as a problem for later.
I also know that there might be alternative approaches like splitting game data between plugins and base game but I think that would make the code far more complex and is not worth the work.

Screenshots

Testing Done

Yes, I disabled a previously enabled plugin, used the button and all the content was gone, did the same in the other direction.

Performance Impact

N/A

Hurleveur · 2024-02-25T11:51:35Z

I think we should have an apply button so it doesnt reload at every click of a plugin?

RisingLeaf · 2024-02-25T11:52:48Z

what do you mean?
The button is the red header it does not reload every time you click something

Hurleveur · 2024-02-25T13:45:18Z

~~UX design is hard, ik, but I thought that was just a msg and not a button since, yk, we dont do buttons like that~~

RisingLeaf · 2024-02-25T13:54:11Z

~~but the a is underlined~~

RisingLeaf · 2024-02-25T13:55:08Z

so you would suggest an extra button next to open plugins?
but thats already where the uninstall button goes

OcelotWalrus · 2024-02-25T16:46:36Z

Maybe a new button at the right of the Open Plugins button.

RisingLeaf · 2024-02-25T16:47:48Z

Maybe a new button at the right of the Open Plugins button.

as I said:

but thats already where the uninstall button goes

OcelotWalrus · 2024-02-25T16:50:49Z

Since when there's an uninstall button?

RisingLeaf · 2024-02-25T16:52:00Z

there is not but I dont want conflicts between my own PRs: #9068

OcelotWalrus · 2024-02-25T16:53:02Z

there is not but I dont want conflicts between my own PRs: #9068

Oh I see! Then maybe on the left of the Back To Menu button?

RisingLeaf · 2024-02-25T16:54:06Z

that feels like the wrong location, but sure

OcelotWalrus · 2024-02-25T16:56:21Z

I know, but that's the most 'non-wrong' free location if we take in count your other PR #9068...

RisingLeaf · 2024-02-25T16:57:49Z

I think I will wait if someone else comes up with a better solution cause editing panels is a pain 😅

OcelotWalrus · 2024-02-25T17:00:58Z

Or maybe instead of a single button in your other PR, you'd add a 'bin' button at the very right of every plugins in the list. That'd fix the compatibility issue.

RisingLeaf · 2024-02-25T17:02:34Z

I dont think that would match ES UI style but I will keep the idea in mind

OcelotWalrus · 2024-02-25T17:02:59Z

I dont think that would match ES UI style but I will keep the idea in mind

True...

Hurleveur · 2024-02-25T17:14:32Z

Cant we just have uninstall to the right (as a plugin specific option) and restart to the left (next to open plugins)?

RisingLeaf · 2024-02-25T17:15:29Z

Ah yes that would make sense

tibetiroka · 2024-02-27T17:14:44Z

This is amazing.

OcelotWalrus · 2024-02-27T17:57:24Z

This is amazing.

Ahah yes.

RisingLeaf · 2024-02-27T19:53:59Z

new layout:

RisingLeaf · 2024-02-27T20:14:53Z

The loading circle now works properly

tibetiroka · 2024-02-28T10:35:32Z

I'm pretty sure I stumbled onto a bug / race condition. I was unloading this plugin and the game just kept showing 'now reloading data' with my cpu at 100%... Most of that is from kernel stuff which makes me think it's either some IO thing or locks not being managed properly.

Interestingly, this issue only appears if I load and then unload the plugin in the same session; it does not happen if it was loaded on startup. It doesn't seem to matter if I do anything between loading and unloading the plugin, though.

So far I have not found any other plugin that I can reproduce the issue with.

OcelotWalrus · 2024-02-28T10:40:01Z

new layout:

The Reload Plugin button ain't proportional to the Open Plugins button?

1010todd · 2024-02-28T14:58:54Z

new layout:

That "Reload to apply changes" text seems to be a little close to the line for some reason?

Also, probably a nitpick but I think just "Reloading plugin data..." without the "Now" would look better.

RisingLeaf · 2024-02-28T18:38:59Z

new layout:

That "Reload to apply changes" text seems to be a little close to the line for some reason?

I did not change that, it has the same position as in main branch

Also, probably a nitpick but I think just "Reloading plugin data..." without the "Now" would look better.

did that 👍

RisingLeaf · 2024-02-28T18:39:44Z

new layout:

The Reload Plugin button ain't proportional to the Open Plugins button?

Ah yes I just used the default wide button ui element, I can also go into gimp and copy paste the open button

thewierdnut · 2024-02-28T18:58:04Z

I'm pretty sure I stumbled onto a bug / race condition. I was unloading this plugin and the game just kept showing 'now reloading data' with my cpu at 100%... Most of that is from kernel stuff which makes me think it's either some IO thing or locks not being managed properly.

I can reproduce this. It seems to be stuck drawing an absurdly large starfield with logical-garbage looking parameters.

When I Re-run using a libasan build, it crashes almost immediately when I click on the Reload button. It claims that the StarField is referencing a sprite that no longer exists:

 ==1146869==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c0008df3b4 at pc 0x556f80c9b3df bp 0x7ffcc0fa52e0 sp 0x7ffcc0fa52d8
READ of size 4 at 0x60c0008df3b4 thread T0
    #0 0x556f80c9b3de in Sprite::Height() const /home/rian/git/endless-sky/source/Sprite.cpp:148
    #1 0x556f8052ee6a in Body::Height() const /home/rian/git/endless-sky/source/Body.cpp:82
    #2 0x556f8052ef0b in Body::Radius() const /home/rian/git/endless-sky/source/Body.cpp:90
    #3 0x556f80cad556 in StarField::Draw(Point const&, Point const&, double, System const*) const /home/rian/git/endless-sky/source/StarField.cpp:227
    #4 0x556f807b5866 in GameLoadingPanel::Draw() /home/rian/git/endless-sky/source/GameLoadingPanel.cpp:96
    #5 0x556f80d3df50 in UI::DrawAll() /home/rian/git/endless-sky/source/UI.cpp:124
    #6 0x556f803d6a8d in GameLoop(PlayerInfo&, Conversation const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) /home/rian/git/endless-sky/source/main.cpp:434
    #7 0x556f803d3d13 in main /home/rian/git/endless-sky/source/main.cpp:221
    #8 0x7fa4ea147249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #9 0x7fa4ea147304 in __libc_start_main_impl ../csu/libc-start.c:360
    #10 0x556f803d2900 in _start (/home/rian/git/endless-sky/build_asan/endless-sky+0x9b900)

Which it said was deleted here:

0x60c0008df3b4 is located 116 bytes inside of 128-byte region [0x60c0008df340,0x60c0008df3c0)
freed by thread T0 here:
    #0 0x7fa4eaaba3c8 in operator delete(void*, unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:164
    #1 0x556f80ca8b6b in std::__new_allocator<std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> > >::deallocate(std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> >*, unsigned long) /usr/include/c++/12/bits/new_allocator.h:158
    #2 0x556f80ca884e in std::allocator_traits<std::allocator<std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> > > >::deallocate(std::allocator<std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> > >&, std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> >*, unsigned long) /usr/include/c++/12/bits/alloc_traits.h:496
    #3 0x556f80ca790e in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> > >::_M_put_node(std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> >*) /usr/include/c++/12/bits/stl_tree.h:565
    #4 0x556f80ca6a55 in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> > >::_M_drop_node(std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> >*) /usr/include/c++/12/bits/stl_tree.h:632
    #5 0x556f80ca6152 in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> > >::_M_erase(std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> >*) /usr/include/c++/12/bits/stl_tree.h:1937
    #6 0x556f80ca612f in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> > >::_M_erase(std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> >*) /usr/include/c++/12/bits/stl_tree.h:1935
    #7 0x556f80ca61a9 in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> > >::clear() /usr/include/c++/12/bits/stl_tree.h:1254
    #8 0x556f80ca58ed in std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, Sprite, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, Sprite> > >::clear() /usr/include/c++/12/bits/stl_map.h:1182
    #9 0x556f80ca4f8c in SpriteSet::Clear() /home/rian/git/endless-sky/source/SpriteSet.cpp:43
    #10 0x556f80728045 in GameData::Clear() /home/rian/git/endless-sky/source/GameData.cpp:224
    #11 0x556f803d3ee9 in main /home/rian/git/endless-sky/source/main.cpp:239
    #12 0x7fa4ea147249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

source/GameData.cpp

tibetiroka · 2024-02-28T19:50:46Z

Is there a way to hide the reload button if there are no changes? Or do we want it to stay there as it could be useful for plugin development?

RisingLeaf · 2024-02-28T19:51:26Z

Or do we want it to stay there as it could be useful for plugin development?

That was my thought

quyykk

You want to quickly disable/enable a plugin but don't like to restart the game every time you switch between plugins?

But if you simply reload everything, what difference is there? You're just saving a couple of clicks on the executable. Is that worth it?

quyykk · 2024-03-22T21:12:24Z

source/Sprite.cpp

+	if(texture[0]) glDeleteTextures(1, &texture[0]);
+	if(texture[1]) glDeleteTextures(1, &texture[1]);
+	if(swizzleMask[0]) glDeleteTextures(1, &swizzleMask[0]);
+	if(swizzleMask[1]) glDeleteTextures(1, &swizzleMask[1]);


Does adding this cause a delay when closing the game? Presumably it will? If so, we might want to manually unload the sprites instead of relying on the destructor.

Is this the case for sounds as well?

I tried it out with high dpi and some other large plugins, and didn't really feel the difference. I didn't have a bunch of custom sounds to try it out with, though.

you should not notice any difference because either you do it here or the OS does it by itself at program end, will be the same result. only difference would be the checks, but I dont think that will make a large difference

quyykk · 2024-03-22T21:15:20Z

source/main.cpp

+
+		// This loop is for the case of a data reload.
+		do {
+			Messenger::SetReload(false);


This is not really a clean way of solving the problem. Probably best to do it using the existing Panel infrastructure instead of drawing this manually here in main. So say like clearing all panels, adding a new panel (kinda similar to GameLoadingPanel) that reloads all the resources.

Maybe you can even reuse GameLoadingPanel by doing some refactoring? Say by moving some common initializing code there instead of doing it in main. I'm not sure though.

source/Audio.cpp

source/UniverseObjects.cpp

1010todd · 2024-03-27T13:16:14Z

You're just saving a couple of clicks on the executable. Is that worth it?
I very often don't have the executable right behind the game + have a lot of folders open so yes, that'd save me a bit of time digging for the exe.

RisingLeaf · 2024-05-02T13:10:11Z

You want to quickly disable/enable a plugin but don't like to restart the game every time you switch between plugins?

But if you simply reload everything, what difference is there? You're just saving a couple of clicks on the executable. Is that worth it?

Actually you save a lot of malloc calls because I just use clear(), which should lead to a quicker loading sequence.

RisingLeaf added 6 commits February 24, 2024 17:08

first steps

9f0413a

this is cool

83af0fc

comments

eeac7aa

moar comments

afd1bab

Merge branch 'master' into texture-storing

2138c4e

style

038df46

Hecter94 added the enhancement A suggestion for new content or functionality that requires code changes label Feb 26, 2024

RisingLeaf added 3 commits February 27, 2024 20:51

looks better ig

6e9222b

Update interfaces.txt

683ae7f

Update interfaces.txt

f36340d

fixes and stuff

849af9a

Merge remote-tracking branch 'upstream/master' into clear-data

1b7a4e2

RisingLeaf added 2 commits February 28, 2024 19:35

Update UniverseObjects.cpp

b9f10db

small text change

a49844a

thewierdnut reviewed Feb 28, 2024

View reviewed changes

source/GameData.cpp Show resolved Hide resolved

RisingLeaf added 3 commits February 28, 2024 20:33

fixed that one

43522ab

important

502373f

get rid of that

186bbec

RisingLeaf mentioned this pull request Mar 14, 2024

Refactor: General purpose TaskQueue instead of SpriteQueue. #6928

Merged

quyykk reviewed Mar 22, 2024

View reviewed changes

RisingLeaf added 2 commits May 3, 2024 15:16

Merge remote-tracking branch 'upstream/master' into clear-data

1fb465c

up to dateness

f5dfb90

Feat(plugins): Reload game data without restarting the game #9868

Are you sure you want to change the base?

Feat(plugins): Reload game data without restarting the game #9868

Conversation

RisingLeaf commented Feb 25, 2024

Summary

Screenshots

Testing Done

Performance Impact

Hurleveur commented Feb 25, 2024

RisingLeaf commented Feb 25, 2024

Hurleveur commented Feb 25, 2024

RisingLeaf commented Feb 25, 2024

RisingLeaf commented Feb 25, 2024 • edited

OcelotWalrus commented Feb 25, 2024

RisingLeaf commented Feb 25, 2024

OcelotWalrus commented Feb 25, 2024

RisingLeaf commented Feb 25, 2024

OcelotWalrus commented Feb 25, 2024

RisingLeaf commented Feb 25, 2024

OcelotWalrus commented Feb 25, 2024

RisingLeaf commented Feb 25, 2024

OcelotWalrus commented Feb 25, 2024 • edited

RisingLeaf commented Feb 25, 2024

OcelotWalrus commented Feb 25, 2024

Hurleveur commented Feb 25, 2024

RisingLeaf commented Feb 25, 2024

tibetiroka commented Feb 27, 2024

OcelotWalrus commented Feb 27, 2024

RisingLeaf commented Feb 27, 2024

RisingLeaf commented Feb 27, 2024

tibetiroka commented Feb 28, 2024 • edited

OcelotWalrus commented Feb 28, 2024

1010todd commented Feb 28, 2024

RisingLeaf commented Feb 28, 2024

RisingLeaf commented Feb 28, 2024

thewierdnut commented Feb 28, 2024

tibetiroka commented Feb 28, 2024

RisingLeaf commented Feb 28, 2024

quyykk left a comment

Choose a reason for hiding this comment

quyykk Mar 22, 2024

Choose a reason for hiding this comment

tibetiroka May 3, 2024

Choose a reason for hiding this comment

RisingLeaf May 3, 2024

Choose a reason for hiding this comment

quyykk Mar 22, 2024

Choose a reason for hiding this comment

1010todd commented Mar 27, 2024

RisingLeaf commented May 2, 2024

RisingLeaf commented Feb 25, 2024 •

edited

OcelotWalrus commented Feb 25, 2024 •

edited

tibetiroka commented Feb 28, 2024 •

edited