Not logged inOpenClonk Forum
Up Topic Development / Developer's Corner / Functions in proplist constants, Effects
1 2 Previous Next
- - By Günther [de] Date 2015-09-27 16:21 Edited 2015-09-27 16:28
Before I went on hiatus, I had almost finished a new feature:
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.
Reply
Parent - - By Zapper [de] Date 2015-09-27 17:42
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 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?
Parent - By Günther [de] Date 2015-09-27 18:46

> 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.
Reply
Parent - By Sven2 [us] Date 2015-09-27 18:45
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).
Parent - By Maikel Date 2015-09-28 00:16
Looks good, let me know when you have the documentation done, then I can do some testing of the new features.
Parent - - By Günther [de] Date 2015-12-23 01:56
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.
Reply
Parent - - By Sven2 [ag] Date 2015-12-23 02:31
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?
Parent - By Günther [de] Date 2015-12-23 18:14
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.
Reply
Parent - - By Günther [de] Date 2015-12-24 00:31
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.
Reply
Parent - - By Zapper [de] Date 2015-12-25 17:28
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.
Parent - By Günther [de] Date 2015-12-28 04:02
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 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.
Reply
Parent - - By Günther [de] Date 2016-01-02 21:54
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?
Reply
Parent - By Maikel Date 2016-01-02 21:58
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.
Parent - By Sven2 [jm] Date 2016-01-02 22:11
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.
Parent - - By Günther [de] Date 2016-04-28 23:35
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
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.
Reply
Parent - - By Marky [de] Date 2016-04-29 19:35
Does that mean that the old scripts have to be changed? I may have some effects on my branch.
Parent - By Luchs [de] Date 2016-04-29 22:39
No, old effects using AddEffect continue to work. It's probably a good idea to write new effects with the new system, though.
Parent - By Anonymous [de] Date 2016-05-13 20:27
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.
Reply
- - By Zapper [de] Date 2016-05-01 17:53
Why is 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!
Parent - - By Luchs [de] Date 2016-05-01 17:58
It's also useful for helper functions associated with an effect.
Parent - By Zapper [de] Date 2016-05-01 18:34
Yes!
Parent - - By Günther [de] Date 2016-05-04 03:18 Edited 2016-05-04 03:20
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 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.
Reply
Parent - - By Günther [de] Date 2016-05-12 02:14

> 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?
Reply
Parent - - By Maikel Date 2016-05-13 11:47
I am doing some experiments. Is there a reason why Rock->CreateEffect is not allowed? ERROR: Effect target has to be an object.
Parent - By Günther [de] Date 2016-05-14 00:11
It is simply not implemented. It'd be somewhat complicated to add because there's no obvious place to store effects on definitions in savegames.
Reply
Parent - - By Maikel Date 2016-05-13 12:02
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?
Parent - - By Luchs [de] Date 2016-05-13 13:16
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.
Parent - - By Maikel Date 2016-05-13 13:23
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
Parent - - By Luchs [de] Date 2016-05-13 13:26
Are proplists defined like this actually duplicated for each instance of the object?
Parent - By Maikel Date 2016-05-13 17:30
I think for each of the object they are a reference pointing to the definition's proplist. But don't quote me on that.
Parent - By Günther [de] Date 2016-05-14 00:14
Nope. They're not even stored in the objects, they simply inherit them from the definition.
Reply
Parent - - By Günther [de] Date 2016-05-14 00:48
Maybe proplist constants should support the former syntax as well. And also ';' instead of ','.
Reply
Parent - By Maikel Date 2016-05-14 07:59
For me it is fine, allowing two different syntaxes is confusing, the current one is reasonable to me.
Parent - - By Maikel Date 2016-05-13 12:56
How do I use functions like GetEffect and GetEffectCount which take a string and not a proplist?
Parent - - By Luchs [de] Date 2016-05-13 13:16
Proplists have a name, see the docs for GetName().
Parent - - By Maikel Date 2016-05-13 13:24
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.
Parent - By Luchs [de] Date 2016-05-13 13:29
Okay. I thought I tried this, but apparently I ended up storing the effect proplist for removal.
Parent - By Günther [de] Date 2016-05-14 00:46
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.
Reply
Parent - By Maikel Date 2016-05-13 13:13
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;
  },
  ...
};
Parent - - By Maikel Date 2016-05-13 13:25 Edited 2016-05-13 13:33
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);
  }
};
Parent - - By Luchs [de] Date 2016-05-13 13:31
This could probably be implemented in script by introducing an Effect prototype object like the one in Guenther's post above.
Parent - By Maikel Date 2016-05-13 13:33
See my edit, I just thought of that as well.
- By Günther [de] Date 2016-05-15 20:54
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.
Reply
- - By Maikel Date 2017-01-20 19:00
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.
Parent - - By Maikel Date 2017-01-20 20:02
Possible fix:

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.
Parent - By Sven2 [us] Date 2017-01-20 22:52
Should be fixed in the engine I think. I don't like hotfixing functions in script.
Parent - - By Günther [de] Date 2017-01-23 01:55 Edited 2017-01-23 02:04
As far as I can tell, the engine already does this.

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.
Reply
Parent - By Maikel Date 2017-01-23 11:51
Seems indeed to already be working. Maybe I made some other mistake somewhere. Thanks.
Parent - - By Sven2 [us] Date 2017-01-20 22:54
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?
Parent - - By Maikel Date 2017-01-21 22:15
Yes, I could adapt the scenario saving, if I can use CommandTarget as a proper identifier. I don't plan to include backwards compatibility and I'll fix the scenarios.
Up Topic Development / Developer's Corner / Functions in proplist constants, Effects
1 2 Previous Next

Powered by mwForum 2.29.7 © 1999-2015 Markus Wichitill