Skip to content

Conversation

@EatThePath
Copy link
Contributor

Music management system for configurable, mission designer controlled dynamic music. Lua version and demo missions included.

Copy link
Contributor

@Goober5000 Goober5000 left a comment

Choose a reason for hiding this comment

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

Looked through this. It's a nice feature and I'm sure Scroll, for one, will be able to use it. I'll have to play around with it.

I spotted a few spelling mistakes but don't let those crowd out the overall pretty positive review.

One important thing though is managing the dependencies. You include several files here from both SendMessageAltSEXPs and PlasmaCore. These files should be removed, and instead Dynamic Jams should specify that these scripts are dependencies. This is similar to how AxBase is listed as a dependency for a lot of things; it's better to maintain one copy of AxBase than several.

@@ -0,0 +1,69 @@
---------------------------------------------------------------
-------- functions for OOP --------
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 a SendMessageAltSEXPs file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, iirc it's required for the demo so I felt it appropriate to include it in the demo. Otherwise anyone attempting to test the demo would need to figure out and add the dependencies used by the mission

Copy link
Contributor

Choose a reason for hiding this comment

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

Otherwise anyone attempting to test the demo would need to figure out and add the dependencies used by the mission

You can assume that anyone using this repository would know how to do that. The entire point of specifying a dependency is to avoid duplicating files in every place that uses them.

Look at PromptBox for example. It depends on AxBase and it will not function without the files in AxBase. Yet the PromptBox demo does not come bundled with the AxBase files.

@@ -0,0 +1,13 @@
#Conditional Hooks

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 a PlasmaCore file

EatThePath and others added 6 commits October 22, 2023 15:30
Co-authored-by: Goober5000 <Goober5000@users.noreply.github.com>
Co-authored-by: Goober5000 <Goober5000@users.noreply.github.com>
Co-authored-by: Goober5000 <Goober5000@users.noreply.github.com>
Co-authored-by: Goober5000 <Goober5000@users.noreply.github.com>
Co-authored-by: Goober5000 <Goober5000@users.noreply.github.com>
Co-authored-by: Goober5000 <Goober5000@users.noreply.github.com>
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.

2 participants