Rulesets for table-based function params#143
Conversation
…functions requiring lists.
| tooltip: How long this particle system should last, 0.0 means forever. | ||
| type: integer | ||
| value: 19 | ||
| value-type: float |
There was a problem hiding this comment.
yes. this is where I was hoping you'd put this data, rather than in the ruleset section
| _TABLE_RULESET_TYPE_MAP: dict[str, str] = { | ||
| "float": "number", | ||
| "integer": "number", | ||
| "vector": "vector", | ||
| "string": "string", | ||
| "key": "(string | uuid)", | ||
| "asset": "(string | uuid)", | ||
| } |
There was a problem hiding this comment.
I'll formalize this better in a future PR. I do like this structure over all the loose -semantics fields laying around
| apply_fn = ruleset_data["apply-fn"] | ||
| apply_link_fn = ruleset_data["apply-link-fn"] |
There was a problem hiding this comment.
this will definitely need improvement at some point (now, later, whatever)
apply_fn has the right signature for
- llCreateCharacter
- llParcelMediaCommandList
- llSetCameraParams
- llSetGroundTexture
- llUpdateCharacter
apply_link_fn doesn't have the right signature for any function except for llLinkParticleSystem, but it's close for these (it's missing face):
- llSetLinkGLTFOverrides
- llSetLinkMedia
- llSetPrimMediaParams
neither one has the right signature for
- llRezObjectWithParams
- llSetAgentEnvironment
- llSetEnvironment
- llCastRay
- llEvade (no params yet)
- llFindNotecardTextCount (no params yet)
- llFindNotecardTextSync (no params yet)
- llFleeFrom (no params yet)
- llGetAgentList (no params yet)
- llGetAttachListFiltered
- llGetClosestNavPoint
- llGetStaticPath
- llGiveAgentInventory
- llHTTPRequest
- llMapBeacon
- llNavigateTo
- llPatrolPoints
- llPursue
- llSetKeyframeMotion
- llSetParcelForSale
- llTransferOwnership (no params yet)
- llWanderWithin
| pattern: | ||
| property: full-write | ||
| type: number | ||
| innerangle: |
There was a problem hiding this comment.
Alias suggestions. I wanted them all to be consistently subject_adjective. I also changed start to begin for consistency with angle_begin, which I think is already in the right form
innerangle->angle_innerouterangle->angle_outerstart_scale->scale_beginend_scale->scale_endstart_color->color_beginend_color->color_endstart_alpha->alpha_beginend_alpha->alpha_endstart_glow->glow_beginend_glow->glow_endinterp_color->color_interpinterp_scale->scale_interp
| @@ -0,0 +1,47 @@ | |||
| // particle-params | |||
| static const FluentParamDescriptor kParticleParamsDescs[] = { | |||
| {"flags", 'i', 0}, | |||
There was a problem hiding this comment.
if both flags and booleans are given, they are ored together?
| flag-mask: _MASK | ||
| lua-module: llparticle | ||
| lua-type: ParticleParams | ||
| apply-fn: ParticleSystem |
There was a problem hiding this comment.
I don't think llParticleSystem needs a lua wrapper. It's covered by a llLinkParticleSystem wrapper that defaults to LINK_THIS. that's what ParamSetter does, I think (but there it's also the only 0-delay variant)
| type: '{[string]: any}?' | ||
| return-type: ParticleParams | ||
| methods: | ||
| - name: apply |
There was a problem hiding this comment.
from Harold:
Is the builder itself necessary there? What utility does it provide over an exported declared type for the dict-like table and a function that accepts tables of that shape? That's generally how similar APIs work in other Luau projects, the "builder-ness" of SPP isn't something that's really needed elsewhere.
That way serialization is easier as well, since you don't need to preserve the type
I don't know if a fluent wrapper is really what we want / need here. Just declaring an expected table shape and a function that consumes it should be fine, since that's really all the builder does
There was a problem hiding this comment.
yeah. that's true. most lua api's would be like
llprim.particleSystem{
emissive = true,
target_linear = true,
pattern = PSYS_SRC_PATTERN_EXPLODE,
start_color = vector(0, 0, 1),
src_max_age = 10,
burst_rate = 0.1,
burst_count = 10,
target_key = ll.GetOwner()
}"just a function" would work better for most of the param-dict wrappers: #143 (comment)
There was a problem hiding this comment.
I'm unsure whether those functions should look like 1.
function llprim.setMedia(link: number, face: number, params: {})to match the lsl function, or like 2.
function llprim.setMedia(params: {}, link: number?, face: number?)so the link/face can be optional (defaulting to LINK_THIS and ALL_SIDES, or
3. mix the two and make a mess like table.insert does:
llprim.setMedia: ((link: number, face: number, params: {}) -> ())
& ((face: number, params: {}) -> ())
& ((params: {}) -> ())or, 4. just add link number/face number as optional dict params, and leave positional params out of it entirely
There was a problem hiding this comment.
oh. the dict parameter should be optional too, and work the same as an empty list. Most of the param-dict functions make sense with an empty param list
| body = "\n".join(desc_lines) | ||
| section = ( | ||
| f"// {ruleset_name}\n" | ||
| f"static const FluentParamDescriptor {array_name}[] = {{\n{body}\n}};\n" |
There was a problem hiding this comment.
Is it still true that the bulk of the code to generate the builder lives in lsl-definitions, and slua/tools/gen_primparams_methods.py calls that code and then converts it to c struct format? just thinking ahead to what changing the structure of rulesets in the yaml file would actually look like given that it's growing in capability
There was a problem hiding this comment.
from Rider:
I believe that yes, gen_primparams_methods is called out of the slua repo.
I'm not entirely happy with that design since it means that slua needs to match the simulator.
That implies that ANY change to lsl_definitions requires a recompile on the VM as well.
(spelling correction to a tooltip? new VM)
There was a problem hiding this comment.
I got the idea somewhere in my mind that there was an intermediate json file that gen_all_definitions wrote and that gen_primparams_methods read, but, I can't find anything like that in reality
would probably be better if that was reality, so that slua only needed the lsl-definitions build, not lsl-definitions itself
but, I can understand why it isn't that way right now. PrimParams isn't even in beta yet, unless I missed it
There was a problem hiding this comment.
from Rider:
It has to do with autobuild and version numbers. build lsl_definitions and the version goes up. Update the simulator and the version no longer matches the one use to build the VM and autobuild screams about version mismatches.
There was a problem hiding this comment.
from Harold:
prior feedback RE the codegen for those wrappers being in slua proper so particles can be mocked / re-implemented for local dev / testing as with the SPP wrapper. There's not really a better place to put them other than slua that keeps the local dev story sane. Ideally people should only have to re-implement the very low-level implementation details of the thing that our wrappers wrap (ll.ParticleSystem here, since that's the thing that does the work) so things like do local simulation a la LSLForge can work correctly without a ton of work on their part.
Generates a lua interface for functions requiring structured lists of parameters with an initial implementation for
ll.ParticleSystemas a POC.Setting to draft for the moment for comments.
related PRs:
secondlife/slua#86
https://github.com/secondlife/server/pull/2574