| Age | Commit message (Collapse) | Author |
|---|
|
Now that all the module setup_funcs are returning a res_setting
w/-EINVAL in res_setup baking, it should be fine for
setup_interactively() to resume using the single setup_func()
loop passing res_setup, and always using res_setting on -EINVAL.
This is especially desirable now that :-prefix settings are
accepted as overrides. You can now get arbitrary values down to
the res_setup baking phase, previously when you got something
wrong there you'd get an ambiguous error without a setting path.
With this commit, you should get a much more useful error
including a setting path.
This partially undoes 5191d68, where res_setup-baking was split
off from the core loop, to occur after the res_failed_desc on
EINVAL storage.
|
|
This was done for rkt_scener already, this commit brings
interactive setup inline WRT :raw_value overrides on multiple
choice settings.
|
|
When creating nested setting instances, just pass down the full
raw value so if there's any prefix on the value it can be
realized as a prefix for the first entry in the nested instance.
|
|
The application of overrides was still done via ad-hoc value
manipulation. This changes to use til_setting_set_raw_value() in
the obvious manner.
|
|
Trivial refactor s/til_setting_spec_check/til_setting_check_spec/
so it operates on a til_setting_t as opposed to the bare value.
With the containing til_setting_t onhand it can be responsible
for bypassing the check when til_setting_t.nocheck is set.
Adjusted callers in setup_interactively() and rkt_scener_update()
accordingly.
|
|
Returning the failed desc was just a lazy half-assed thing that
was sort of the best option in the simpler, pre-paths world.
But now that everything has paths thanks to recursive settings,
let's just return the path to the failed setting's desc.
This conveniently gets rid of a UAF bug when setup returned the
setting->desc as the failed desc, and main would print the desc
*after* freeing all the settings in its final moments.
But the best part is now more of the errors parsing settings
should be accompanied by an illuminatingly relevant setting path.
Previously you'd at best get a bare key from the desc, but often
no failed desc was returned at all and you saw no guidance at
all. But with the recent improvements to the setup error
handling I think those cases should be few if not entirely
eliminated.
|
|
This is just temporary until all the setup_funcs always return a
res_setting on -EINVAL.
Currently they only cover this requirement when res_setup is
NULL...
|
|
This eliminates more open_memstream() usage in favor of the new
til_str stuff.
While here I've split up the path construction API exposing the
til_str oriented variants, rather than only providing a
FILE*-oriented API. This way you just use the FILE* stuff when
convenient (like printing to stdout/stderr) and go til_str_t when
you're building up a buffer.
Such is life in a sans-open_memstream-world.
Also while here the FILE*-oriented settings path printers were
renamed s/print/fprint/g hence touching setup.c
|
|
Preparatory commit for bridging the gap separating a baked
til_setup_t from a runtime-populated descendant til_settings_t
like modules::rtv produces for its channels via
til_module_setup_randomize().
For these currently orphaned til_settings_t instances we don't
readily have access to the logical ancestor til_settings_t that
was used to produce the module_context's bound til_setup_t. But
we don't really need the ancestor til_settings_t, all we _really_
want is the ancestral path to prefix the orphan til_settings_t
instances.
So this commit introduces supplying a prefix which gets prepended
to paths printed via the settings instance. A later commit will
make use of this in modules::rtv when producing the settings
instance passed to til_module_setup_randomize()
|
|
In a world where "describing" settings is an iterative process,
especially post-nested-settings which are realized via the
desc-applying process, it's better to not even offer desc-setting
while adding a new setting.
This commit just gets rid of that.
The one caller that was passing a non-NULL desc to
til_settings_add_value(), til_module_setup_randomize(), was
redundantly doing so since the subsequent desc-processing was
assigning it again anyways. Future commits will likely change
til_module_setup_randomize() use a non-NULL desc for skipping
desc-applying, which wouldn't even work if it was always setting
the desc @ add time. That becomes necessary for partially
randomizing sparsely-populated settings.
|
|
In situations where modules wish to alias setting values like
expanding "all" -> "mod0,mod1,mod2,mod3" they need a way to
intercept the value-acceptance @ desc-assignment time in the
front-end. This optional override() function does just that when
present in the spec.
The current setting's value is passed to the override, and
if what's returned differs from what was passed (by pointer
value), then the current value is freed and the override takes
its place. The override function is expected to _always_ return
non-NULL; either the value provided, or a newly allocated value
override. The override function must never free the supplied
value, that's the front-end's job in applying the override.
The override() must return NULL on errors, which are assumed to
be limited to ENOMEM failures.
|
|
This commit introduces setting path printing to interactive
setup, making it much more clear what on earth you're setting up
in more complex scenarios.
e.g. an interactive setup of module=compose is now:
```
$ src/rototiller --module=compose
/module/compose/layers:
Comma-separated list of module layers, in draw-order [drizzle,stars,spiro,plato]:
/module/compose/layers/[0]/drizzle/viscosity:
Puddle viscosity:
0: .005
1: .01
2: .03
3: .05
Enter a value 0-3 [1 (.01)]:
/module/compose/layers/[0]/drizzle/style:
Overlay style:
0: mask
1: map
Enter a value 0-1 [0 (mask)]:
/module/compose/layers/[1]/stars/rot_adj:
Rotation rate:
0: .0
1: .00001
2: .00003
3: .0001
4: .0003
5: .001
Enter a value 0-5 [2 (.00003)]:
/module/compose/layers/[3]/plato/orbit_rate:
Orbit rate and direction:
0: -1
1: -.75
2: -.5
3: -.25
4: .1
5: 0
6: .1
7: .25
8: .5
9: .75
10: 1
Enter a value 0-10 [7 (.25)]:
/module/compose/layers/[3]/plato/spin_rate:
Spin rate and direction:
0: -1
1: -.75
2: -.5
3: -.25
4: .1
5: 0
6: .1
7: .25
8: .5
9: .75
10: 1
Enter a value 0-10 [9 (.75)]:
/module/compose/texture:
Module to use for source texture, "none" to disable:
0: none
1: blinds
2: checkers
3: drizzle
4: julia
5: moire
6: plasma
7: roto
8: stars
9: submit
10: swab
11: voronoi
Enter a value 0-11 [0 (none)]: 1
/module/compose/texture/blinds/orientation:
Blinds orientation:
0: horizontal
1: vertical
Enter a value 0-1 [0 (horizontal)]:
/module/compose/texture/blinds/count:
Blinds count:
0: 2
1: 4
2: 8
3: 12
4: 16
5: 24
6: 32
Enter a value 0-6 [4 (16)]: 3
/video:
Video backend:
0: drm
1: mem
2: sdl
Enter a value 0-2 [2 (sdl)]:
/video/sdl/fullscreen:
SDL fullscreen mode:
0: off
1: on
Enter a value 0-1 [0 (off)]:
/video/sdl/size:
SDL window size [640x480]:
Configured settings as flags:
--seed=0x6471f827 '--module=compose,layers=drizzle\\\,viscosity\\\=.01\\\,style\\\=mask\,stars\\\,rot_adj\\\=.00003\,spiro\,plato\\\,orbit_rate\\\=.25\\\,spin_rate\\\=.75,texture=blinds\,orientation\=horizontal\,count\=12' '--video=sdl,fullscreen=off,size=640x480'
Press enter to continue, add --go to skip this step...
```
Previously it would be like so:
$ src/rototiller --module=compose
Comma-separated list of module layers, in draw-order [drizzle,stars,spiro,plato]:
Puddle viscosity:
0: .005
1: .01
2: .03
3: .05
Enter a value 0-3 [1 (.01)]:
Overlay style:
0: mask
1: map
Enter a value 0-1 [0 (mask)]:
Rotation rate:
0: .0
1: .00001
2: .00003
3: .0001
4: .0003
5: .001
Enter a value 0-5 [2 (.00003)]:
Orbit rate and direction:
0: -1
1: -.75
2: -.5
3: -.25
4: .1
5: 0
6: .1
7: .25
8: .5
9: .75
10: 1
Enter a value 0-10 [7 (.25)]:
Spin rate and direction:
0: -1
1: -.75
2: -.5
3: -.25
4: .1
5: 0
6: .1
7: .25
8: .5
9: .75
10: 1
Enter a value 0-10 [9 (.75)]:
Module to use for source texture, "none" to disable:
0: none
1: blinds
2: checkers
3: drizzle
4: julia
5: moire
6: plasma
7: roto
8: stars
9: submit
10: swab
11: voronoi
Enter a value 0-11 [0 (none)]: 1
Blinds orientation:
0: horizontal
1: vertical
Enter a value 0-1 [0 (horizontal)]:
Blinds count:
0: 2
1: 4
2: 8
3: 12
4: 16
5: 24
6: 32
Enter a value 0-6 [4 (16)]: 3
Video backend:
0: drm
1: mem
2: sdl
Enter a value 0-2 [2 (sdl)]:
SDL fullscreen mode:
0: off
1: on
Enter a value 0-1 [0 (off)]:
SDL window size [640x480]:
Configured settings as flags:
--seed=0x6471f827 '--module=compose,layers=drizzle\\\,viscosity\\\=.01\\\,style\\\=mask\,stars\\\,rot_adj\\\=.00003\,spiro\,plato\\\,orbit_rate\\\=.25\\\,spin_rate\\\=.75,texture=blinds\,orientation\=horizontal\,count\=12' '--video=sdl,fullscreen=off,size=640x480'
Press enter to continue, add --go to skip this step...
```
Which gets rather confusing, especially if you go even deeper
with nested module situations like
compose/layers/[0]/checkers/fill_module/compose/layers/[0]/drizzle
You just lose track of which module instance's settings you're
actually setting up
This is just the first step of making use of these paths. In the
future the module context paths will be derived from the settings
paths so they're more reliably distinct and descriptive, along
with consistent with paths in the settings namespace. That
becomes important when module contexts start getting registered
on the stream for use in other modules.
|
|
In this "all settings require descs" world, when setup_func
forgets to return a res_desc for a setting it wants, we can end
up in an infinite loop situation. It's better to abort
immediately on the assert to catch such a program error.
|
|
Since 1a8abe80dabd this ceased to be the right thing to do, since
there's no longer the whole bare-key setting with null value.
It's now always a value, and the key that's optional.
|
|
Preparatory for constructing unique paths from a given
setting/settings instance by walking up the tree
|
|
It'll be perfectly normal to turn bare-value settings int nested
instances. In such scenarios we don't have a static
spec-supplied key for the label, so let's just generate a label
like a C-style array subscript for such instances.
|
|
This commit pivots everything over to using desc->container as
the target settings instance when adding settings, as well as
actually assigning the settings container @ desc create.
Given nothing is actually triggering settings heirarchies yet (no
specs set as_nested_settings) this shouldn't actually result in
any realized functional difference, yet. The settings pointer
being placed in desc->container should be identical to what was
getting used before.
|
|
For recursive settings the individual setting being described
needs to get added to a potentially different settings instance
than the one being operated on at the top of the current
setup_func phase.
The settings instance being passed around for a setup_func to
operate on is constified, mainly to try ensure modules don't
start directly mucking with the settings. They're supposed to
just describe what they want next and iterate back and forth,
with the front-end creating the settings from the returned descs
however is appropriate, eventually building up the settings to
completion.
But since it's the setup_func that decides which settings
instance is appropriate for containing the setting.. at some
point it must associate a settings instance with the desc it's
producing, one that is going to be necessarily written to.
So here I'm just turning the existing til_setting_desc_t to a
"spec", unchanged. And introducing a new til_setting_desc_t
embedding the spec, accompanied by a non-const til_settings_t*
"container".
Now what setup_funcs use to express settings are a spec,
otherwise identically to before. Instead of cloning a desc to
allocate it for returning to the front-end, the desc is created
from a spec with the target settings instance passed in.
This turns the desc step where we take a constified settings
instance and cast it into a non-const a more formal act of going
from spec->desc, binding the spec to a specific settings
instance. It will also serve to isolate that hacky cast to a
til_settings function, and all the accessors of
til_setting_desc_t needing to operate on the containing settings
instance can just do so.
As of this commit, the container pointer is just sitting in the
desc_t but isn't being made use of or even assigned yet. This is
just to minimize the amount of churn happening in this otherwise
mostly mechanical and sprawling commit.
There's also been some small changes surrounding the desc
generators and plumbing of the settings instance where there
previously wasn't any. It's unclear to me if desc generators
will stay desc generators or turn into spec generators. For now
those are mostly just used by the drm_fb stuff anyways, modules
haven't made use of them, so they can stay a little crufty
harmlessly for now.
|
|
The core thing here is rather than turning a bare value into a
key as I was doing before - we just leave the bare value as a
bare value and its setting must be located positionally via
get_value_by_idx since there's no key.
Existing callers that used to get_key() positionally now
get_value_by_idx() positionally all the same, except it's the
value instead of the key. This is mostly done for things like
the module or fb name at the front of a settings instance.
The impetus for this change is partially just
cosmetic/ergonomics, but it's also rather strange for what's
really a key-less value to be treated as a value-less key. It
was also awkward to talk/reason about on the road to recursive
settings where bare values would be supported as a standalone
settings instance if properly escaped...
This also adds unescaping of keys, and adds a dependency on the
somewhat linux-specific open_memstream() which may need changing
in the future (see comments).
|
|
setup_func isn't formally defined for libtil, but
setup_interactively() defacto establishes it and
til_module_t.setup() reflects the same signature and calling
convention except with til_settings_t constified.
This change makes them all consistent in this regard, but there
should probably be a formal typedef added for the function.
The reason for constifying this is I don't want setup functions
directly manipulating the settings instance. In the case of
rototiller::setup_interactively() we ensure the stdio-based
interactive setup is always the side doing the manipulation of
the settings. For a libtil-user like glimmer, it's slightly
different beast with GTK+ in the loop, but by preventing the
setup_funcs from messing directly with the settings (instead
having to describe what they want done iteratively), the
front-end always gets its opportunity to maintain its state while
doing the described things.
Of course, this is mostly a lie, and within libtil the constified
til_settings_t gets cast away to modify it in places. But
keeping that limited to within libtil is tolerable IMO. We just
don't want to see such casts in module code.
|
|
When this is set, the setting is itself to be a settings instance
that the frontend must create and place in the relevant
til_setting_t.value_as_nested_settings.
This commit implements that frontend portion in
setup_interactively() for the rototiller frontend.
No setup_func() yet attempts to make use of this stuff. There's
probably more change needed before that can happen, specifically
the setup_func() likely must always produce a til_settings_t* to
indicate which settings instance is currently relevant to the
frontend. Without setup_func() telling the frontend, the
frontend has basically no other way of knowing when the backend
setup_func() has moved up/down the heirarchy at the current
iteration.
|
|
If you hit ^D during interactive setup it'd send things
infinitely spinning.
This commit treats eof when expecting more input as -EIO and
simply gives up. Which I imagine technically means it's possible
to terminate the last interactive question with EOF/^D instead of
newline and have it work, since we only check it before the
fgets() used to get more input.
|
|
This commit improves the error printed when cli-supplied args
fail, adding at least the key name to what used to be just a
stringified errno:
```
$ src/rototiller --module=shapes,scale=99
Shape type:
0: circle
1: pinwheel
2: rhombus
3: star
Enter a value 0-3 [1 (pinwheel)]:
Fatal error: unable to use args for setting "scale": Invalid argument
$
```
|
|
I don't think rototiller is an appropriate place for being so
uncooperative, if someone gets the case wrong anywhere just make
it work. We should avoid making different things so subtly
different that case alone is the distinction anyways, so I don't
see this creating any future namespace collision problems.
|
|
This brings something resembling an actual type to the private
objects returrned in *res_setup. Internally libtil/rototiller
wants this to be a til_setup_t, and it's up to the private users
of what's returned in *res_setup to embed this appropriately and
either use container_of() or casting when simply embedded at the
start to go between til_setup_t and their private containing
struct.
Everywhere *res_setup was previously allocated using calloc() is
now using til_setup_new() with a free_func, which til_setup_new()
will initialize appropriately. There's still some remaining work
to do with the supplied free_func in some modules, where free()
isn't quite appropriate.
Setup freeing isn't actually being performed yet, but this sets
the foundation for that to happen in a subsequent commit that
cleans up the setup leaks.
Many modules use a static default setup for when no setup has
been provided. In those cases, the free_func would be NULL,
which til_setup_new() refuses to do. When setup freeing actually
starts happening, it'll simply skip freeing when
til_setup_t.free_func is NULL.
|
|
This is a preparatory commit for cleaning up the existing sloppy
global-ish application of settings during the iterative _setup()
call sequences.
Due to how this has evolved from a very rudimentary thing
enjoying many assumptions about there ever only being a single
module instance being configured by the settings, there's a lot
of weirdness and inconsistency surrounding module setup WRT
changes being applied instantaneously to /all/ existing and
future context's renderings of a given module vs. requiring a new
context be created to realize changes.
This commit doesn't actually change any of that, but puts the
plumbing in place for the setup methods to allocate and
initialize a private struct encapsulating the parsed and
validated setup once the settings are complete. This opaque
setup pointer will then be provided to the associated
create_context() method as the setup pointer. Then the created
context can configure itself using the provided setup when
non-NULL, or simply use defaults when NULL.
A future commit will update the setup methods to allocate and
populate their respective setup structs, adding the structs as
needed, as well as updating their create_context() methods to
utilize those setups.
One consequence of these changes when fully realized will be that
every setting change will require a new context be created from
the changed settings for the change to be realized.
For settings appropriately manipulated at runtime the concept of
knobs was introduced but never finished. That will have to be
finished in the future to enable more immediate/interactive
changing of settings-like values appropriate for interactive
manipulation
|
|
Now that til_setting_t.desc is not only a thing, but a thing that
is intended to be refreshed regularly in the course of things
like GUI interactive settings construction, it's not really
appropriate to try even act like this these are const anymore.
|
|
The existing iterative *_setup() interface only described
settings not found, quietly accepting usable settings already
present in the til_settings_t.
This worked fine for the existing interactive text setup thing,
but it's especially problematic for providing a GUI setup
frontend.
This commit makes it so the *_setup() methods always describe
undescribed settings they recognize, leaving the setup frontend
loop calling into the *_setup() methods to both apply the
description validation if wanted and actually tie the description
to respective setting returned by the _setup() methods as being
related to the returned description.
A new helper called til_settings_get_and_describe_value() has
been introduced primarily for use of module setup methods to
simplify this nonsense, replacing the til_settings_get_value()
calls and surrounding logic, but retaining the til_setting_desc_t
definitions largely verbatim.
This also results in discarding of some ad-hoc
til_setting_desc_check() calls, now that there's a centralized
place where settings become "described" (setup_interactively in
the case of rototiller).
Now a GUI frontend (like glimmer) would just provide its own
setup_interactively() equivalent for constructing its widgets for
a given *_setup() method's chain of returned descs. Whereas in
the past this wasn't really feasible unless there was never going
to be pre-supplied settings.
I suspect the til_setting_desc_check() integration into
setup_interactively() needs more work, but I think this is good
enough for now and I'm out of spare time for the moment.
|
|
Largely mechanical rename of librototiller -> libtil, but
introducing a til_ prefix to all librototiller (now libtil)
functions and types where a rototiller prefix was absent.
This is just a step towards a more libized librototiller, and til
is just a nicer to type/read prefix than rototiller_.
|
|
This seems unnecessary and doesn't always read well in combination
with a given setting name string.
Just get rid of it for now.
|
|
|
|
Preliminary means for interactively configuring settings and defaults
|
