Fix macos build and run issues

[Skosulor]

trafficstars

The goal of this pull request is to fix build and run issues for macOs.

• Run presenter in the main thread (macos starts a hissy fit unless GUI related stuff is not run in the main thread)
• Remove guards stopping QT events to be processed on macos
• Fix shaders that for an unknown reason crashes when glLinkProgram is called.

Note: Requires some additional tweeks

[Skosulor]

Changing the structure of engine.loop have resulted in some new issues that need to be fixed, error output:

INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/frigate/frigate.nyan
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/destroyer/heavy_destroyer.nyan
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/frigate/advanced_frigate.nyan
 ERR file:///Users/hell/repos/openage/assets/test/qml/main.qml:5 module "yay.sfttech.openage" is not installed

INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/destroyer/destroyer.nyan
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/cruiser/advanced_cruiser.nyan
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/anti_air_destroyer/heavy_anti_air_destroyer.nyan
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/anti_air_destroyer/anti_air_destroyer.nyan

FATAL: terminate has been called

uncaught exception

std::exception of type std::__1::system_error: mutex lock failed: Invalid argument

current stack:

Traceback (most recent call last):
  File ?, in _pthread_start+0x88 [0x1a005c2e4]
  File ?, in void* std::__1::__thread_proxy[abi:ne200100]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0>>(void*)+0x54 [0x103168358]
  File ?, in void std::__1::__thread_execute[abi:ne200100]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0>&, std::__1::__tuple_indices<...>)+0x1c [0x1031686a0]
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/anti_air_destroyer/projectiles/anti_air_destroyer_projectiles.nyan
  File ?, in decltype(std::declval<openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0>()()) std::__1::__invoke[abi:ne200100]<openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0>(openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0&&)+0x18 [0x103168704]
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/troop_center/troop_center.nyan
  File ?, in openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0::operator()() const+0x28 [0x103168760]
  File ?, in openage::time::TimeLoop::run()+0x40 [0x1035bb708]
  File ?, in openage::time::Clock::update_time()+0x218 [0x1035bad20]
  File ?, in std::__1::unique_lock<std::__1::shared_mutex>::~unique_lock[abi:ne200100]()+0x1c [0x1031c4e14]
  File ?, in std::__1::shared_mutex::unlock[abi:ne200100]() [0x1031d3b80]
  File ?, in std::terminate()+0x6c [0x1a00116b4]
  File ?, in std::__terminate(void (*)())+0x10 [0x1a0011710]
  File ?, in openage::error::terminate_handler()+0x278 [0x103170b90]

handing over to the system...

[Skosulor]

I set breakpoints around at destructors and got:

INFO [T2] Loading .nyan file: swgb_base/data/tech/generic/fusion_extractor/fusion_extractor.nyan
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/prefab_shelter/prefab_shelter.nyan
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/shipyard/shipyard.nyan
 ERR file:///Users/hell/repos/openage/assets/test/qml/main.qml:5 module "yay.sfttech.openage" is not installed

Process 42228 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 4.1
    frame #0: 0x00000001024e4864 libopenage.0.dylib`openage::engine::Engine::~Engine(this=0x000000016fdfd898) at engine.h:88:20
   85           Engine &operator=(const Engine &) = delete;
   86           Engine(Engine &&) = delete;
   87           Engine &operator=(Engine &&) = delete;
-> 88           ~Engine() = default;
   89  
   90  
   91           /**
Target 0: (run) stopped.

Not sure how to interpret this. What can I do to debug main.qml?

[heinezen]

You can ignore the QML error. It has nothing to do with this. It only happens because our GUI is currently deactivated.

[Skosulor]

I'm a bit dumbfounded, when the deconstructor for Engine is called I have the following backtrace:

 thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
    frame #0: 0x00000001024e4864 libopenage.0.dylib`openage::engine::Engine::~Engine(this=0x000000016fdfd898) at engine.h:88:20
   85           Engine &operator=(const Engine &) = delete;
   86           Engine(Engine &&) = delete;
   87           Engine &operator=(Engine &&) = delete;
-> 88           ~Engine() = default;
   89  
   90  
   91           /**
Target 0: (run) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  * frame #0: 0x00000001024e4864 libopenage.0.dylib`openage::engine::Engine::~Engine(this=0x000000016fdfd898) at engine.h:88:20
    frame #1: 0x00000001024e457c libopenage.0.dylib`openage::run_game(args=0x000000016fdfddf8) at main.cpp:61:1

Which means that we have come to the end of run_game(). But I also have breakpoints inside the engine.loop function just after the presentation.run() is called.

void Engine::loop() {

	// if presenter is used, run the simulation in a separate thread
	if (this->run_mode == mode::FULL) {

		this->threads.emplace_back([&]() {
			this->loop_simulation();
		});

		this->presenter->run(this->window_settings);
		log::log(MSG(info) << "presenter exited");       
		// Make sure that the presenter gets destructed in the same thread
		// otherwise OpenGL complains about missing contexts
		this->presenter.reset();
		this->running = false;
	}
	else {
		this->loop_simulation();
	}
}

And the execution is never stopped inside the loop() function, nor is log::log(MSG(info) << "presenter exited"); executed.

So it seems that the presenter never exits.

Which lead me to debugging the presenter again, and it seems I ever get past init_graphics(), specifically the call to

	this->gui = std::make_shared<renderer::gui::GUI>(
		this->gui_app, // Qt application wrapper
		this->window,  // window for the gui
		qml_root_file, // entry qml file, absolute path.
		qml_root,      // directory to watch for qml file changes
		qml_assets,    // qml data: Engine *, the data directory, ...
		this->renderer // openage renderer
	);

Seem to halt the execution of the thread, at least I never get past it before the errors. I tried to remove main.qml to see what happens.

I would expect an error thrown from:

	this->gui = std::make_shared<renderer::gui::GUI>(
		this->gui_app, // Qt application wrapper
		this->window,  // window for the gui
		qml_root_file, // entry qml file, absolute path.
		qml_root,      // directory to watch for qml file changes
		qml_assets,    // qml data: Engine *, the data directory, ...
		this->renderer // openage renderer
	);

But instead I see an endless spam of errors, see the gist: https://gist.github.com/Skosulor/0ca46ceeb1c160d88ec0582cce25d1b2

It continues like that for like a couple hundred of thousand lines.

I found that it is actually in the log message when getting the path:

std::string Path::resolve_native_path_w() const {
	auto resolved_path = this->fsobj->resolve_w(this->parts);

	if (resolved_path.first) {
		return resolved_path.second.get_native_path();
	}
	else {
		throw Error{ERR << "failed to locate writable path: " << *this};
	}

I end up in the else clause. I find it a bit weird that I get so much output when an error is thrown.

Anyway, it seems to me that the error I receive: ERR file:///Users/hell/repos/openage/assets/test/qml/main.qml:5 module "yay.sfttech.openage" is not installed

Is related to the presenter failing to initialize graphics as qml_root_file is the main.qml file which imports yay.sfttech.openage

EDIT:

Nevermind yay.sfttech.openage is not the issue, I just removed the import and I got no error but the same result.

The main thread does not seem to get passed the:

	this->gui = std::make_shared<renderer::gui::GUI>(
		this->gui_app, // Qt application wrapper
		this->window,  // window for the gui
		qml_root_file, // entry qml file, absolute path.
		qml_root,      // directory to watch for qml file changes
		qml_assets,    // qml data: Engine *, the data directory, ...
		this->renderer // openage renderer
	);

Code inside presenter.cpp

[Skosulor]

Okay I think I found it, it seems to be more shader problems.

Actually crashing on: auto maptex_shader = this->renderer->add_shader({id_shader_src, maptex_frag_shader_src}); In void GUI::initialize_render_pass

and digging deeper I found that the shader compilation fails:

As seen in the screenshot above GLShader::GLShader fails to compile the shader and for somereason macos does not like exceptions and I get the output seen in the gist earlier.

To conclude my debugging I guess there are two issues

1. The shaders (identity and/or maptexture?)
2. Handling of expection (on macos) which results in a spam of error messages of which none is the actual error

[heinezen]

Can you comment the whole of GUI init and rendering out in the presenter? Maybe this fixes it. The GUI is not drawing anything anyway currently.

https://github.com/SFTtech/openage/blob/86d05c44e93c537e53703db829e72b0b4d06041e/libopenage/presenter/presenter.cpp#L169

and

https://github.com/SFTtech/openage/blob/86d05c44e93c537e53703db829e72b0b4d06041e/libopenage/presenter/presenter.cpp#L316

[heinezen]

I think the issue is that the shader code is too old for the shaders in the GUI. They are based on OpenGL 1.2...

[Skosulor]

I think the issue is that the shader code is too old for the shaders in the GUI. They are based on OpenGL 1.2...

Yup, I just found it! And you are correct, I updated them and that part works now.

However now I think there are a new issue..

INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/frigate/frigate.nyan
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/destroyer/heavy_destroyer.nyan
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/frigate/advanced_frigate.nyan
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/destroyer/destroyer.nyan
INFO Created OpenGL shader program

FATAL: terminate has been called

uncaught exception

std::exception of type std::__1::system_error: mutex lock failed: Invalid argument

current stack:

Traceback (most recent call last):
  File ?, in _pthread_start+0x88 [0x1a005c2e4]
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/cruiser/advanced_cruiser.nyan
  File ?, in void* std::__1::__thread_proxy[abi:ne200100]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0>>(void*)+0x54 [0x102ec8424]
  File ?, in void std::__1::__thread_execute[abi:ne200100]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0>&, std::__1::__tuple_indices<...>)+0x1c [0x102ec876c]
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/anti_air_destroyer/heavy_anti_air_destroyer.nyan
  File ?, in decltype(std::declval<openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0>()()) std::__1::__invoke[abi:ne200100]<openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0>(openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0&&)+0x18 [0x102ec87d0]
  File ?, in openage::engine::Engine::Engine(openage::engine::Engine::mode, openage::util::Path const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, openage::renderer::window_settings const&)::$_0::operator()() const+0x28 [0x102ec882c]
INFO [T2] Loading .nyan file: swgb_base/data/game_entity/generic/anti_air_destroyer/anti_air_destroyer.nyan
  File ?, in openage::time::TimeLoop::run()+0x40 [0x10331ff40]
  File ?, in openage::time::Clock::update_time()+0x218 [0x10331f558]
  File ?, in std::__1::unique_lock<std::__1::shared_mutex>::~unique_lock[abi:ne200100]()+0x1c [0x102f24ea8]
  File ?, in std::__1::shared_mutex::unlock[abi:ne200100]() [0x102f33c14]
  File ?, in std::terminate()+0x6c [0x1a00116b4]
  File ?, in std::__terminate(void (*)())+0x10 [0x1a0011710]
  File ?, in openage::error::terminate_handler()+0x278 [0x102ed0c24]

[heinezen]

We should be almost there. The last renderer sytem that is initialized after the GUI is the final render pass that composes all other renderers. However, that has a very simple shader.

[Skosulor]

Looks like its failing to set unform texture because it does not exist? GUI::initialize_render_pass:

	this->texture_unif = maptex_shader->new_uniform_input("texture", this->texture);

this->texture is confirmed to have been set so I do not know how to interpret the error.

[heinezen]

@Skosulor The error refers to a uniform in the shader source code. What that basically means is: It tried to set a value for the uniform named "texture", but the GLSL source code does not have a uniform with that name.

Edit: The uniform is here in the GLSL code: https://github.com/SFTtech/openage/blob/86d05c44e93c537e53703db829e72b0b4d06041e/assets/shaders/maptexture.frag.glsl#L5

[heinezen]

You might be able to upgrade the shader to OpenGL 3.3 with this code. I did not test this:

#version 330
// total basic standard texture drawing fragment shader

// the texture data
uniform sampler2D texture;

// interpolated texture coordinates received from vertex shader
in vec2 tex_position;
out vec4 col;

void main (void) {
	// this sets the fragment color to the corresponding texel.
	col = texture2D(texture, tex_position);
}

[Skosulor]

You might be able to upgrade the shader to OpenGL 3.3 with this code. I did not test this:

#version 330
// total basic standard texture drawing fragment shader

// the texture data
uniform sampler2D texture;

// interpolated texture coordinates received from vertex shader
in vec2 tex_position;
out vec4 col;

void main (void) {
	// this sets the fragment color to the corresponding texel.
	col = texture2D(texture, tex_position);
}

I made it to:

#version 330 core

uniform sampler2D texture_;

in vec2 tex_position;
out vec4 frag_color;

void main(void) {
    frag_color = texture(texture_, tex_position);
}

So I changed it to texture_ too in the code so now I get even further! But maybe the shader should be as you have written it?

But now I get a new error but I do not think it is the shader anymore!

INFO [T2] Loading .nyan file: swgb_base/data/terrain/path4/path4.nyan
INFO [T2] Game simulation started
INFO Presenter: Graphics subsystems initialized
INFO Presenter: Initializing input subsystem...
INFO Loading game simulation controls
INFO Loading GUI controls
INFO Loading camera controls
INFO Loading HUD controls
INFO Presenter: Input subsystem initialized
INFO [T1] Time loop exited

SIGSEGV

FATAL: terminate has been called

current stack:


SIGSEGV
libc++abi: terminating
Traceback (most recent call last):
  File ?, in _pthread_start+0x88 [0x1a005c2e4]

[Skosulor]

Hmm It seems a thread is trying to access a shared ptr that doesn't exist anymore

thread #6, stop reason = EXC_BAD_ACCESS (code=1, address=0x10)
    frame #0: 0x00000001026e0f6c libopenage.0.dylib`std::__1::shared_ptr<openage::event::State>::shared_ptr[abi:ne200100]<openage::gamestate::GameState, 0>(this=0x00000001700b6ec0, __r=nullptr) at shared_ptr.h:479:87
    frame #1: 0x00000001026d95c0 libopenage.0.dylib`std::__1::shared_ptr<openage::event::State>::shared_ptr[abi:ne200100]<openage::gamestate::GameState, 0>(this=0x00000001700b6ec0, __r=nullptr) at shared_ptr.h:479:119
    frame #2: 0x00000001026d933c libopenage.0.dylib`openage::gamestate::GameSimulation::run(this=0x000000010c204098) at simulation.cpp:49:46
    frame #3: 0x00000001026237fc libopenage.0.dylib`openage::engine::Engine::loop_simulation(this=0x000000016fdfd898) at engine.cpp:84:20
    frame #4: 0x00000001026260e0 libopenage.0.dylib`openage::engine::Engine::loop()::$_0::operator()(this=0x0000600000b2cf98) const at engine.cpp:67:10
    frame #6: 0x000000010262606c libopenage.0.dylib`void std::__1::__thread_execute[abi:ne200100]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, openage::engine::Engine::loop()::$_0>(__t=size=2, (null)=__tuple_indices<> @ 0x00000001700b6f7f) at thread.h:199:3
    frame #7: 0x0000000102625e84 libopenage.0.dylib`void* std::__1::__thread_proxy[abi:ne200100]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, openage::engine::Engine::loop()::$_0>>(__vp=0x0000600000b2cf90) at thread.h:208:3

[heinezen]

INFO [T1] Time loop exited

frame #2: 0x00000001026d933c libopenage.0.dylib`openage::gamestate::GameSimulation::run(this=0x000000010c204098) at simulation.cpp:49:46

Based on this line I think it's more likely that the time loop got destroyed somehow.

[Skosulor]

INFO [T1] Time loop exited

frame #2: 0x00000001026d933c libopenage.0.dylib`openage::gamestate::GameSimulation::run(this=0x000000010c204098) at simulation.cpp:49:46

Based on this line I think it's more likely that the time loop got destroyed somehow.

It seems to happen because the destructor is called on most thread.

I have narrowd down the crash to:

			for (auto &cb : this->on_resize) {
				cb(width, height, this->scale_dpr);

in GlWindow::Update in window.cpp:126

[Skosulor]

Eureka!

Had to update the name of texture in one more place :P

[Skosulor]

Is the performance as seen in the GIF as expected from the current state of openage?

[heinezen]

Performance as in frames? :D You can test that with stresstest 0.

If the question is if that is how openage works in general, then yes, what you see looks like the correct state.

[Skosulor]

Performance as in frames? :D You can test that with stresstest 0.

If the question is if that is how openage works in general, then yes, what you see looks like the correct state.

Yeah, I meant as in frames :P

It seems I cant run it

[heinezen]

Try this

cd bin && ./run test --demo renderer.tests.stresstest 0

[Skosulor]

./run test --demo renderer.tests.stresstest 0

Same result:

[heinezen]

Looks like the docs are wrong :see_no_evil: This command should work:

cd bin && ./run test --demo renderer.tests.renderer_stresstest 0

[Skosulor]

Looks like the docs are wrong 🙈 This command should work:

cd bin && ./run test --demo renderer.tests.renderer_stresstest 0

That went fine, 1-2k FPS ish. But when launching the "full game" I experience a lot of lag when moving around the camera and poor response when zooming in and out as seen in the gif above

[heinezen]

Do the units move normally (i.e. are the animations smooth)?

[Skosulor]

Do the units move normally (i.e. are the animations smooth)?

I have not figured out how to move them :P

[heinezen]

Select them with the rectangle you draw with left mouse, then right click somewhere :D

[Skosulor]

Select them with the rectangle you draw with left mouse, then right click somewhere :D

The GIF is an accurate representation of how it looks for me

[Skosulor]

But I think I'm done with the PR so it is ready for review now :)

[heinezen]

I'm going to think about a few suggestion how to structure the code and then come back with a review :)