Before I went on hiatus, I had almost finished a new feature:
The only missing steps are finishing the documentation, testing, and review.
I've merged the branch with the current master and pushed the result to here.
The commits actually changing the features exposed to Scripters are prefixed with "Script:", so they can be easily distinguished from implementation details. I'm pretty confident in the implementation details, but somebody with fresh knowledge about what the scripts are doing needs to check whether the API changes break stuff.
static const ExampleEffect = {
func Start() { Log("ExampleEffect started!!!"); }
}
func CreateExampleEffect() {
CreateEffect(ExampleEffect, nil, 1);
}
The only missing steps are finishing the documentation, testing, and review.
I've merged the branch with the current master and pushed the result to here.
The commits actually changing the features exposed to Scripters are prefixed with "Script:", so they can be easily distinguished from implementation details. I'm pretty confident in the implementation details, but somebody with fresh knowledge about what the scripts are doing needs to check whether the API changes break stuff.
That probably means that you are breaking all existing effects anyway, right? Or is this going to be kept in addition to the existing stuff?
If you want to break stuff anyway, I'd like to separate the temp-calls from the non-temp calls. I guess nearly all our effects' Start and Stop callbacks start with an
Another question, in which context are the scripts executed? I guess I cannot attach an effect to an object and use
What happens if I add a
Oh, and do we still want to call them effects? They are used for general timers more often :) Sven!
PS: I just noticed that I did not comment about the actual change at all: I like it :)
I can see that we are headed more and more into a direction where we can have single variables contain huge (measured in lines of code) local definitions (ActMap, effects, Particles, ..). What about allowing different scripts for an object which can contain functions/local variables to allow for more modular code in the places where we really have many lines of code?
If you want to break stuff anyway, I'd like to separate the temp-calls from the non-temp calls. I guess nearly all our effects' Start and Stop callbacks start with an
if (!temp) return;
. What about adding Construction/Destruction callbacks that are really only called once each? And Start and Stop would still be called as usual.Another question, in which context are the scripts executed? I guess I cannot attach an effect to an object and use
this
to reference the object anymore?What happens if I add a
Timer
function to an object and say AddEffect(this, ...)
?Oh, and do we still want to call them effects? They are used for general timers more often :) Sven!
PS: I just noticed that I did not comment about the actual change at all: I like it :)
I can see that we are headed more and more into a direction where we can have single variables contain huge (measured in lines of code) local definitions (ActMap, effects, Particles, ..). What about allowing different scripts for an object which can contain functions/local variables to allow for more modular code in the places where we really have many lines of code?
> That probably means that you are breaking all existing effects anyway, right? Or is this going to be kept in addition to the existing stuff?
No, I am not insane. CreateEffect creates effects that call callbacks on the effect, AddEffect works like it used to (except that you can't change the Prototype of effects anymore.)
Calling a different Callback for temp-calls for the new effects would be feasible.
> Another question, in which context are the scripts executed? I guess I cannot attach an effect to an object and use this to reference the object anymore?
this
is the effect, and the object it is operating on a parameter.
Yes, great change and yes, the temp calls should be separated. We may also want priority "nil" to default to 1 (which has no temp calls).
Looks good, let me know when you have the documentation done, then I can do some testing of the new features.
I've rebased the branch on current master, and added Construction and Destruction callbacks. I'm not sure what to do about the temporary parameter to Start() - has anyone checked for the case where an effect is temporarily started again because it got removed while temporarily stopped? Though with the Construction callback, the additional parameters to Start are probably superfluous, so the temporary parameter doesn't get in the way.
Good job! Extra parameters are potentially annoying because they push extra parameters passed through CreateEffect away.
Since we use temp calls approximately never, effects will now almost always use Construction/Destruction with Start/Stop being needed only for the temp stuff?
Since we use temp calls approximately never, effects will now almost always use Construction/Destruction with Start/Stop being needed only for the temp stuff?
Yes. Construction gets just the target and then the extra CreateEffect parameters, while Start also gets the temporary parameter. The whole thing is already simplified by one parameter because there are now only two proplists involved: Target and effect instead of target, commandtarget and effect. Perhaps we should compensate by adding another free-form parameter to CreateEffect? Oh, and I should change CreateEffect to use this() as the target instead of a parameter, since we can use Global->CreateEffect() or Scenario->CreateEffect() now.
Next design problem: The priority of local variables in functions declared in constant proplists.
For top-level-functions, there is a list of local variables, and functions are stored in the proplist. When parsing an identifier, it is looked up in these lists:
1. function parameter
2. function-local variable (var)
3. local variable
4. true, false, nil, new, if, while, else, for, return, Par, this, inherited, _inherited
5. functions (global functions are in the proplist via the prototype)
6. global variables
7. global constants
This order is pretty sane - the usual order from most specific to least specific, you can override keywords with code in the immediate vicinity, but not globally. The different rules for local functions and local variables are a bit odd, but not practically relevant. Except that I now have to decide where to put local variables in constant proplists. And whether to unify lookup of functions and variables for them.
For top-level-functions, there is a list of local variables, and functions are stored in the proplist. When parsing an identifier, it is looked up in these lists:
1. function parameter
2. function-local variable (var)
3. local variable
4. true, false, nil, new, if, while, else, for, return, Par, this, inherited, _inherited
5. functions (global functions are in the proplist via the prototype)
6. global variables
7. global constants
This order is pretty sane - the usual order from most specific to least specific, you can override keywords with code in the immediate vicinity, but not globally. The different rules for local functions and local variables are a bit odd, but not practically relevant. Except that I now have to decide where to put local variables in constant proplists. And whether to unify lookup of functions and variables for them.
Hmmm, I may not understand the issue. A constant proplist would not have "real" (object) local variables, right? So in the lookup order, they would just replace the number 3? And the lookup order would probably traverse the proplist up to see whether it can find that name?
I might have completely misunderstood you, though.
I might have completely misunderstood you, though.
Yes, number 3 doesn't apply to functions in constant proplists. However, looking in the proplist finds functions as well as other values, so that isn't a one-to-one replacement for local variables. There are several wacky solutions:
a) Check for all properties first, then keywords. You can always replace
b) Check for keywords first, then properties. You can sometimes replace
c) Check the value in the property. Extra code to provide consistency with an historical accident.
d) Change the rules for all functions. I'm not sure where the best place for the keywords is - I like the compatibility aspect of allowing old code that uses a new keyword as an ordinary identifier to continue to work, so I'd tend to place them after functions. If we place them at the end, the difference between a global constant and
a) Check for all properties first, then keywords. You can always replace
true
with a local variable, and sometimes with a function.b) Check for keywords first, then properties. You can sometimes replace
true
with a local variable, and never with a function.c) Check the value in the property. Extra code to provide consistency with an historical accident.
d) Change the rules for all functions. I'm not sure where the best place for the keywords is - I like the compatibility aspect of allowing old code that uses a new keyword as an ordinary identifier to continue to work, so I'd tend to place them after functions. If we place them at the end, the difference between a global constant and
true
, false
and nil
shrinks, which looks tidy. On the other hand, a scenario with a global constant with the same name as a new keyword will probably break anyway. Also, the list of forbidden keywords is somewhat arbitrary, and should probably be checked at the point of declaration instead of usage.
So I'm finally changing the effects documentation to use CreateEffect, and its making me question the wisdom of adding Scenario effects: Having two global effect lists makes it possible for global effects that should stack to miss each other.
Context: The "CreateEffect" style always has a target object. This can be an object, the proplist in the global constant Global, or the proplist in the global constant Scenario. The target object will be the hidden this parameter of CreateEffect, so in the scenario script a plain CreateEffect will create an effect with the Scenario proplist as target parameter. This makes scenario scripts more similar to object scripts, which is nice. On the other hand, global effects must be created with Global->CreateEffect outside of a handfull of engine callbacks, and whether to use Scenario or Global in that instance is pretty much arbitrary.
Should I use a single effect list for both Scenario and Global, so that stacking global effects always use the same stack, while priority=1 effects don't even notice the difference? Or only allow Scenario as target for CreateEffect? How are global effects even used in practice?
Context: The "CreateEffect" style always has a target object. This can be an object, the proplist in the global constant Global, or the proplist in the global constant Scenario. The target object will be the hidden this parameter of CreateEffect, so in the scenario script a plain CreateEffect will create an effect with the Scenario proplist as target parameter. This makes scenario scripts more similar to object scripts, which is nice. On the other hand, global effects must be created with Global->CreateEffect outside of a handfull of engine callbacks, and whether to use Scenario or Global in that instance is pretty much arbitrary.
Should I use a single effect list for both Scenario and Global, so that stacking global effects always use the same stack, while priority=1 effects don't even notice the difference? Or only allow Scenario as target for CreateEffect? How are global effects even used in practice?
My only use for global effects was in scenario scripts, because there is no other way. But some examples of global effects outside the scenario scope are in System.ocg/Schedule.c.
Global effects are used e.g. when there is no target object (I think disasters like earthquakes use a global effect). Another use is when an effect should outlast an object (like particle effects after an object disappeared).
The main use case for scenario effects would be to have callbacks into the scenario. At the moment, this is not possible. You can use global schedule with a pointer (ScheduleCall(nil, Scenario.foo)), but then the this-pointer is not the scenario. This is used for stuff specific to the scenario like waterfalls or ensuring that trees or fish respawn even if all of them died, etc. They're also used in conjunction with LoadScenarioSection because loading another section during the object list execution causes a crash. Global/Scenario effects also needs to make sure that you can do LoadScenarioSection from their timer callback.
The main use case for scenario effects would be to have callbacks into the scenario. At the moment, this is not possible. You can use global schedule with a pointer (ScheduleCall(nil, Scenario.foo)), but then the this-pointer is not the scenario. This is used for stuff specific to the scenario like waterfalls or ensuring that trees or fish respawn even if all of them died, etc. They're also used in conjunction with LoadScenarioSection because loading another section during the object list execution causes a crash. Global/Scenario effects also needs to make sure that you can do LoadScenarioSection from their timer callback.
Finally merged this.
http://docs.openclonk.org/en/sdk/script/Effects.html
http://docs.openclonk.org/en/sdk/script/fn/CreateEffect.html
Of course, the next day Luchs discovers a design problem with the functions in proplist constants, namely that proplists not inheriting from Global (as all object definitions and the scenario do) don't inherit global functions. So the example really needs to be
Of course, we could add a new global, say "Effect", which derives from Global, so that we can replace "new Global" with "new Effect". I think I intended that to be the solution, but I forgot to actually implement it or write it into the examples.
Alternatively, change the parser to also look up global functions, even if the proplist owning the function doesn't have them. That'd make plain Log() etc. work, but effect->Log() or effect->GetName() wouldn't work.
http://docs.openclonk.org/en/sdk/script/Effects.html
http://docs.openclonk.org/en/sdk/script/fn/CreateEffect.html
Of course, the next day Luchs discovers a design problem with the functions in proplist constants, namely that proplists not inheriting from Global (as all object definitions and the scenario do) don't inherit global functions. So the example really needs to be
static const ExampleEffect = new Global {
func Start() { Log("ExampleEffect started!!!"); }
}
func CreateExampleEffect() {
Global->CreateEffect(ExampleEffect, 1);
}
Of course, we could add a new global, say "Effect", which derives from Global, so that we can replace "new Global" with "new Effect". I think I intended that to be the solution, but I forgot to actually implement it or write it into the examples.
Alternatively, change the parser to also look up global functions, even if the proplist owning the function doesn't have them. That'd make plain Log() etc. work, but effect->Log() or effect->GetName() wouldn't work.
Does that mean that the old scripts have to be changed? I may have some effects on my branch.
Couldn't one even use Properties to categorize Effects now? The per-name categories like eg *PSpell, *Fire* depend on their position in the effect name, thus are not arbitrarily combinable. As Effects are proplist defined before Effect creation, one could do sth like this:
var eff = new Global {
Construction = ...,
Timer = ...,
IsFire = true,
IsSpell = true,
}
and implement a function like GetEffectByProperty (or extend GetEffect). With this one you could do GetEffectByProperty("IsFire", ...) instead of GetEffect("*Fire*", ...).
But the effects proplist would need to be passed to th Effect-Callback of effects of higher priority, to be really useful. It seems a bit strange to get a effect passed you may not interact with. But values that are not yet initialized may simply be nil, or it may be "undefined" to access them. The values defined in the proplist to create the effect from would be accessible.
var eff = new Global {
Construction = ...,
Timer = ...,
IsFire = true,
IsSpell = true,
}
and implement a function like GetEffectByProperty (or extend GetEffect). With this one you could do GetEffectByProperty("IsFire", ...) instead of GetEffect("*Fire*", ...).
But the effects proplist would need to be passed to th Effect-Callback of effects of higher priority, to be really useful. It seems a bit strange to get a effect passed you may not interact with. But values that are not yet initialized may simply be nil, or it may be "undefined" to access them. The values defined in the proplist to create the effect from would be accessible.
Why is
I think it would look a lot nicer, if target would just be a property of the effect. It would also not be much more to type (fx.Target) but instead remove having to add one obligatory argument everywhere (ideally even when it's not needed as a hint to the next coder about how the signature of the call really looks to remove possible error sources).
TL;DR: I think
Target
not a property of the effect (like CommandTarget
) but passed as an argument everywhere?I think it would look a lot nicer, if target would just be a property of the effect. It would also not be much more to type (fx.Target) but instead remove having to add one obligatory argument everywhere (ideally even when it's not needed as a hint to the next coder about how the signature of the call really looks to remove possible error sources).
TL;DR: I think
Target
belongs to the effect properties instead of the argument list - would be cleaner and more unified!
It's also useful for helper functions associated with an effect.
Because effects didn't store that information, and this whole project required far too many patches as is. Case in point: I let myself be persuaded and wrote some patches to do this, and ended up spending most of the time upgrading the StdCompiler to consistently support two parameters for CompileFuncs. I've pushed the results on the script branch, but don't have any time left today to test this or write documentation. For all I know it'll crash loading some DefCore.txt or something.
If one adds
If one adds
static const Effect = new Global { Target = nil; };
and uses that as effect prototype prototype, then plain Target should work, no need for effect.Target. I think that's also entirely untested, though.
> I've pushed the results on the script branch
And later a version that actually compiles. At least with gcc. Anyone want to test the branch?
I am doing some experiments. Is there a reason why Rock->CreateEffect is not allowed? ERROR: Effect target has to be an object.
The documentation of Effects is currently quite a chaos and hard to understand.
Which is the correct syntax?
func Stop(object target, int reason, bool temporary) {}
or
Construction = func(object target) {}
Also what happened to the CommandTarget? i.e. where can I define an effect and where is it available?
Which is the correct syntax?
func Stop(object target, int reason, bool temporary) {}
or
Construction = func(object target) {}
Also what happened to the CommandTarget? i.e. where can I define an effect and where is it available?
The second one is correct, although without the target parameter on Guenther's branch. There is no command target anymore as the effect functions are all defined on the proplist you pass to CreateEffect.
So also an object's local proplist persists even if the object is removed? I have the example with local InvisPSpell in mind over here: http://docs.openclonk.org/en/sdk/script/Effects.html
Are proplists defined like this actually duplicated for each instance of the object?
Maybe proplist constants should support the former syntax as well. And also ';' instead of ','.
How do I use functions like GetEffect and GetEffectCount which take a string and not a proplist?
GetEffectCount(ScheduleCallEffect->GetName(), this) did not work for me, moreover that is a very ugly way to find the effect of the number of effects.
Okay. I thought I tried this, but apparently I ended up storing the effect proplist for removal.
Yeah, the effectnames are broken. I probably was imagining that they'd inherit their name from their prototype if I made GetName() return the local variable name where constant proplists are stored, but of course that's not how C++ inheritance works.
I basically want the upsides of automatically adding a Name property to constant proplists with the variable name, without the downsides of cluttering every random constant proplist with an extra property.
For now, I've made new effects store the name in a property just like the old effects do.
I basically want the upsides of automatically adding a Name property to constant proplists with the variable name, without the downsides of cluttering every random constant proplist with an extra property.
For now, I've made new effects store the name in a property just like the old effects do.
If I give my effect a property Name, it does not work anymore.
static const ScheduleCallEffect = new Global
{
Name = "ScheduleCallEffect",
Construction = func(call_function, int repeats, array pars)
{
// Init effect variables.
this.call_function = call_function;
this.repeats = repeats;
this.pars = pars;
},
...
};
static const ScheduleCallEffect = new Global
{
Name = "ScheduleCallEffect",
Construction = func(call_function, int repeats, array pars)
{
// Init effect variables.
this.call_function = call_function;
this.repeats = repeats;
this.pars = pars;
},
...
};
Maybe we can have something like effect->Remove() instead of RemoveEffect(), as easier syntax?
Something like this even works:
static const Effect = new Global
{
Target = nil,
Remove = func(bool no_calls)
{
return RemoveEffect(nil, this.Target, this, no_calls);
}
};
Something like this even works:
static const Effect = new Global
{
Target = nil,
Remove = func(bool no_calls)
{
return RemoveEffect(nil, this.Target, this, no_calls);
}
};
I've now merged the target-as-property-instead-of-parameter change. Also, arrays constant are finally read-only and no longer a ticking time bomb for savegames.
It seems that EffectCall does not work on the new type of effects. Is this true? If so scenario saving is broken for those effects.
In particular I am reworking the AI a bit where this is needed.
In particular I am reworking the AI a bit where this is needed.
Possible fix:
Any objections to this? I don't see a proper way to test whether an effect is new.
global func EffectCall(object target, effect fx, string fn, ...)
{
// Implement effect call for the new effects which do not have a command target.
if (!fx.CommandTarget)
return fx->Call(Format("~%s", fn), ...);
return _inherited(target, fx, fn, ...);
}
Any objections to this? I don't see a proper way to test whether an effect is new.
Should be fixed in the engine I think. I don't like hotfixing functions in script.
As far as I can tell, the engine already does this.
This logs "Pass" twice.
The presence of CommandTarget is how the engine distinguishes the effect variants. I was using the presence of a prototype at first, and I could image changing to C++ inheritance or something, but I doubt the new variant will re-use CommandTarget for something else. That'd be far too confusing.
local A = { Custom=func() { Log("Pass"); } };
local B = { Custom=func(a) { this.s = a; } };
func Main() {
var e = CreateEffect(A, 1);
EffectCall(nil, e, "Custom");
e = CreateEffect(B, 1);
EffectCall(nil, e, "Custom", "Pass");
Log(e.s);
}
This logs "Pass" twice.
The presence of CommandTarget is how the engine distinguishes the effect variants. I was using the presence of a prototype at first, and I could image changing to C++ inheritance or something, but I doubt the new variant will re-use CommandTarget for something else. That'd be far too confusing.
Btw, don't you know if an effect is old style or new style? You could just update the EffectCalls to save to the more sensible fx->fn(...) directly.
Or is it for backwards compatibility with scenarios that have AI enemies?
Or is it for backwards compatibility with scenarios that have AI enemies?
Powered by mwForum 2.29.7 © 1999-2015 Markus Wichitill