modules/rkt: utilize til_module_setup_full()

rkt_setup() and rkt_scener_update() had distinct implementations
for scene module setup.  This consolidates that where trivial to
both use the new til_module_setup_full() with appropriate
parameters, wrapped up in rkt_scene_module_setup().

The finalizing phase is still ad-hoc which is mildly annoying,
but if finalizing just passed into rkt_scene_module_setup() there
wouldn't be the til_module_t onhand for sticking in rkt_scene_t.

So the code to extract and lookup the module from the settings
would still be needed anyways, as the whole til setup_func api
isn't limited to modules so the baked til_setup_t doesn't come
back with a til_module_t hanging in there.  Maybe in the future
this gets changed a bit, there could for instance be a void* in
til_setup_t where something usage-specific goes, like the
relevant module in the case of a module's setup.  Something to
consider for the future.

Consolidating these in the pre-finalize phase at least ensures
consistent behavior in initial rkt::scenes setup vs. scener
editing/new scenes.

3 files changed, 26 insertions, 36 deletions

diff --git a/src/modules/rkt/rkt.c b/src/modules/rkt/rkt.c
index ba8b3ab..263da23 100644
--- a/src/modules/rkt/rkt.c
+++ b/src/modules/rkt/rkt.c
@@ -27,6 +27,8 @@
  * GNU Rocket (https://github.com/rocket/rocket)
  */
 
+#define RKT_DEFAULT_SCENE_MODULE	":blank"
+
 /* variadic helper wrapping librocket's sync_get_track() */
 static const struct sync_track * rkt_sync_get_trackf(rkt_context_t *ctxt, const char *format, ...)
 {
@@ -382,6 +384,19 @@ static void rkt_setup_free(til_setup_t *setup)
 }
 
 
+int rkt_scene_module_setup(const til_settings_t *settings, til_setting_t **res_setting, const til_setting_desc_t **res_desc, til_setup_t **res_setup)
+{
+	return til_module_setup_full(settings,
+				     res_setting,
+				     res_desc,
+				     res_setup,
+				     "Scene Module",
+				     RKT_DEFAULT_SCENE_MODULE,
+				     (TIL_MODULE_EXPERIMENTAL | TIL_MODULE_HERMETIC),
+				     NULL); /* << "rkt" would be wise, but it already gets caught by HERMETIC */
+}
+
+
 static int rkt_setup(const til_settings_t *settings, til_setting_t **res_setting, const til_setting_desc_t **res_desc, til_setup_t **res_setup)
 {
 	const til_settings_t	*scenes_settings;
@@ -402,14 +417,11 @@ static int rkt_setup(const til_settings_t *settings, til_setting_t **res_setting
 	const char		*listen_port;
 	int			r;
 
-	/* This is largely taken from compose::layers, but might just go away when I add tables to rocket,
-	 * or maybe they can coexist.
-	 */
 	r = til_settings_get_and_describe_value(settings,
 						&(til_setting_spec_t){
 							.name = "Comma-separated list of modules for scenes to sequence",
 							.key = "scenes",
-							.preferred = ":blank", /* FIXME TODO: this should really be NULL or "" for no scenes at all, but that doesn't work yet */
+							.preferred = RKT_DEFAULT_SCENE_MODULE, /* FIXME TODO: this should really be NULL or "" for no scenes at all, but that doesn't work yet */
 							.annotations = NULL,
 							.as_nested_settings = 1,
 						},
@@ -440,37 +452,12 @@ static int rkt_setup(const til_settings_t *settings, til_setting_t **res_setting
 		}
 
 		for (size_t i = 0; til_settings_get_value_by_idx(scenes_settings, i, &scene_setting); i++) {
-			til_setting_t		*scene_module_setting;
-			const char		*scene_module_name = til_settings_get_value_by_idx(scene_setting->value_as_nested_settings, 0, &scene_module_setting);
-			const til_module_t	*scene_module;
-
-			if (!scene_module_name || !scene_module_setting->desc) {
-				r = til_setting_desc_new(	scene_setting->value_as_nested_settings,
-								&(til_setting_spec_t){
-									.name = "Scene module name",
-									.preferred = "none",
-									.as_label = 1,
-								}, res_desc);
-				if (r < 0)
-					return r;
-
-				*res_setting = scene_module_name ? scene_module_setting : NULL;
-
-				return 1;
-			}
-
-			scene_module = til_lookup_module(scene_module_name);
-			if (!scene_module) {
-				*res_setting = scene_module_setting;
-
-				return -EINVAL;
-			}
-
-			if (scene_module->setup) {
-				r = scene_module->setup(scene_setting->value_as_nested_settings, res_setting, res_desc, NULL);
-				if (r)
-					return r;
-			}
+			r = rkt_scene_module_setup(scene_setting->value_as_nested_settings,
+						   res_setting,
+						   res_desc,
+						   NULL); /* XXX: note no res_setup, must defer finalize */
+			if (r)
+				return r;
 		}
 	}
 
diff --git a/src/modules/rkt/rkt.h b/src/modules/rkt/rkt.h
index 53b9eba..e5ed4dc 100644
--- a/src/modules/rkt/rkt.h
+++ b/src/modules/rkt/rkt.h
@@ -61,4 +61,7 @@ typedef struct rkt_setup_t {
 	rkt_setup_scene_t	scenes[];
 } rkt_setup_t;
 
+
+int rkt_scene_module_setup(const til_settings_t *settings, til_setting_t **res_setting, const til_setting_desc_t **res_desc, til_setup_t **res_setup);
+
 #endif
diff --git a/src/modules/rkt/rkt_scener.c b/src/modules/rkt/rkt_scener.c
index 6b3f8bb..c272caa 100644
--- a/src/modules/rkt/rkt_scener.c
+++ b/src/modules/rkt/rkt_scener.c
@@ -993,7 +993,7 @@ int rkt_scener_update(rkt_context_t *ctxt)
 	case RKT_SCENER_FSM_SEND_NEWSCENE_SETUP: {
 		int	r;
 
-		r = til_module_setup(scener->new_scene.settings,
+		r = rkt_scene_module_setup(scener->new_scene.settings,
 				     &scener->new_scene.cur_setting,
 				     &scener->new_scene.cur_desc,
 				     NULL);	/* res_setup deliberately left NULL for two reasons: