![[de]](/mwf/flags/de.png "Germany")
Here's a first round of patches to remove the old physical implementation. There's even a new feature in there: You can now change the acceleration by script, instead of modifying the engine.There are still some physicals left: Energy, Breath, Jump, Throw, CorrosionResist and BreatheWater. I'm not sure what to do with them. The last four can probably just be simple properties. Maybe the first two can be properties named MaxEnergy and MaxBreath or something. When that is done, SetPhysical and GetPhysical can be removed.
I probably made some mistakes while modifying the scripts. Comments and test help are welcome. :-)
So I tried them and it didn't compile at first hand, I think you may have forgotten to remove C4TempPhysicalInfo::Train. I removed that and it compiled.
In general movement seems like it didn't change, so that's a good thing - your conversions seem correct. But from reading your conversion changes I get the feeling that horizontal movement while jumping with a clonk is faster than just walking, seems a little strange to me. Was this already the case before your conversions, or did you make some error there?
P.S. got some crashes with while using the grappler, compiling a debug engine now.
In general movement seems like it didn't change, so that's a good thing - your conversions seem correct. But from reading your conversion changes I get the feeling that horizontal movement while jumping with a clonk is faster than just walking, seems a little strange to me. Was this already the case before your conversions, or did you make some error there?
P.S. got some crashes with while using the grappler, compiling a debug engine now.
> But from reading your conversion changes I get the feeling that horizontal movement while jumping with a clonk is faster than just walking, seems a little strange to me. Was this already the case before your conversions, or did you make some error there?
The flight movement had a hardcoded maximum of 2px/frame. I'd blame Newton, but the real culprit is the convoluted Physical code that made doing something "better" unnecessary difficult. And noticing the difference between 1.96px/frame and 2 px/frame is probably impossible anyway.
edit:
> So I tried them and it didn't compile at first hand, I think you may have forgotten to remove C4TempPhysicalInfo::Train. I removed that and it compiled.
Partially - I removed it in another commit that removes C4TempPhysicalInfo completely, but moved that commit to the end and didn't push it because some scripts still rely on it. I'll amend the "training" commit, thanks.
>When that is done, SetPhysical and GetPhysical can be removed.
Wait, how would I for example change the walking speed of a Clonk then? Modifying the ActMap-proplist?
Then we will probably need some easy-to-use helper function that provide some sort of stack mechanism, because modifying the walking speed is a common practice and actually in some original packs from CR you can get the error that when you combine two different slow effects, you will be faster than before when they wear off
You get one guess as to how I modified the existing scripts to still modify the walk speed.
A decent stack function that respects priorities and percentage values and actually applies changes retroactively when a change with a lower priority is added while still being able to remove single changes by accessing them via some sort of ID? :)
I'm no fan of excessive complexity where something simpler works, so I just implemented a "push a new speed on the stack" and "pop the last pushed speed off the stack". If two scripts try to interfere with the speed in a non-nested order, that can lead to wrong speeds, but as long as the number of pushes and pops are equal, no permanent damage will be done. And instead of making this API more complicated, I'd recommend using the stacking support of the Effects, which was implemented for just this usecase. If that's too complicated, develop suggestions for making it simpler.
The usual problem was- you have a speed of 100
- you are slowed by 50%
- you have a speed of 50
- you are slowed by 10
- you have a speed of 40
- the first slow (-50%) wears of
- you are at 80
- the second slow (-10) wears off
- you are at 90
Problem!
>I'd recommend using the stacking support of the Effects, which was implemented for just this usecase.
Mh, true. Even though I have a solution in mind, it would in the end be not much different from properly using effect priorities/temporary calls
That's why you're storing the previous value instead of applying the inverse function. While you're then at 50 instead of at 90 or whatever the desired result would be after the first slow wears off, at the end you're at the expected 100.
This doesn't work eitherYou're at 100
First slow applies. Stores previous value (100) and reduces to 90
Second slow applies. Stores previous value (90) and reduces to 80
First slow wears off. Resets speed to stored 100
Second slow wears off. Resets speed to stored 90
-> Both slows gone, you end up at 90 speed
The only way to prevent this is to use a stack that ensures the first applied slow is removed last. I.e., effects.
Okay, I think we can use that implementation until someone notices it's not working as intended and posts a bug in the bugtracker :]
I don't thinkStart: PushStack(...)
Stop: PopStack(...)
is much simpler than
Start: x = PushStack(...)
Stop: RemoveFromStack(x)
and the latter would have the advantage of having the correct physicals between the removes. Remember some physical modifiers can work in a very long time frame. For example, think of boots you can equip to make you faster. If you equip those boots while a two-second temporary slow effect had hit you, you will be slowed forever (until you re-equip them).
You don't store the previous value, but the absolute value of the modification * (-1), determined when the effect starts:- 100
- Reduce by 10 (save +10) -> 90
- Reduce by 10% (save +9) -> 81
- Increase by 200% (save -162) -> 243
- Increase by 2 (save -2) -> 245
It's unimportant in what order these effects end, the outcome is always 245 - 2 - 162 + 9 + 10 = 100
That produces obviously wrong results, though.100 - double effect, save -100
200 - three quarters effect, save +150
50 - double effect ends
-50 - oops
Really, the way Effects work by temporarily removing and reapplying changes is the only way to ensure totally correct values. Short of doing that, using the previous value will at least result in usable values with no longterm harm.
![[pl]](/mwf/flags/pl.png "Poland")
1) Apply value modifiers(+- X)
2) Apply percentage modifiers(+- X%)
This way:
- you have a speed of 100
- you are slowed by 50%
- you have a speed of 50
- you are slowed by 10
- you have a speed of 45 (if it's a problem, the steps could always get reversed...)
- the first slow (-50%) wears of
- you are at 90
- the second slow (-10) wears off
- you are at 100
Voila!
2) Apply percentage modifiers(+- X%)
This way:
- you have a speed of 100
- you are slowed by 50%
- you have a speed of 50
- you are slowed by 10
- you have a speed of 45 (if it's a problem, the steps could always get reversed...)
- the first slow (-50%) wears of
- you are at 90
- the second slow (-10) wears off
- you are at 100
Voila!
We could make an "modifier" list for some values (Speed mostly). E.g. that everytime an modifier is added or removed the speed is recalculated.
It starts with 100.
Effect A adds an 200% modifier. (Modifiers now: 200%, Calculation: 100*2.0=200)
Effect B adds an 50% modifier. (Modifiers: 200% 50%, Calculation: 100*2.0*0.5=100)
Effect C adds an +50 modifier. (Modifiers: 200% 50% +50, Calculation: 100*2.0*0.5+50=150)
Effect A runs out. Removes the 200% modifier. (M: 50% +50, C: 100*0.5+50=100)
Effect C runs out. Removes the +50 modifier. (M: 50% C: 100*0.5=50)
Effect B runs out. Removes the 50% modifier. (M: Nothing C: 100=100)
However dunno what API could be used for that.
It starts with 100.
Effect A adds an 200% modifier. (Modifiers now: 200%, Calculation: 100*2.0=200)
Effect B adds an 50% modifier. (Modifiers: 200% 50%, Calculation: 100*2.0*0.5=100)
Effect C adds an +50 modifier. (Modifiers: 200% 50% +50, Calculation: 100*2.0*0.5+50=150)
Effect A runs out. Removes the 200% modifier. (M: 50% +50, C: 100*0.5+50=100)
Effect C runs out. Removes the +50 modifier. (M: 50% C: 100*0.5=50)
Effect B runs out. Removes the 50% modifier. (M: Nothing C: 100=100)
However dunno what API could be used for that.
What I had in mind was some sort of unique identifier for every slow (some object number, probably) that can be used to remove certain effects. Additionally a priority and the effect itself would have to be saved - and of course everything would be recalculated every time a new slow is applied
> What I had in mind was some sort of unique identifier for every slow
You mean the effect number? Really, stop describing the Effect API we already have. It was designed for this and can solve the problem. We only have to make it usable. Help with that would be really appreciated!
Then the problem is that effects can only be added to objects but not to values ;)
But yeah, that could work.
+ Some code to handle those effects.
But yeah, that could work.
func Fx*ValModifierStart([...], string property)
{
EffectVar(0, [...]) = property;
}
func Fx*ValModifierIsForProperty([...], string property)
{
return EffectVar(0, [...]) == property;
}
func Fx*ValModifierCalculate([...], value)
{
return value; // +25 or something, dependant what kind of modifier
}+ Some code to handle those effects.
>Really, stop describing the Effect API we already have.
Let me quote myself: "Even though I have a solution in mind, it would in the end be not much different from properly using effect priorities/temporary calls".
I am also for improving the effect interface for this task.
>easy-to-use helper function that provide some sort of stack mechanism
That's easy:
DoMovementSpeed(int change); will suffice I think (assuming that the normal function/act map property is called SetMovementSpeed/movementSpeed).
As this is not directly related to designing achievements / the removal or redesign of trainable physicals, I'd promote this branch to an own topic with your permission.
Here are some patches I made after our discussion in IRC, SetMaxEnergy does not really work cause of C4Object.cpp line 1370, If you can fix that, please do so, otherwise I'll have a look tomorrow.
Attachment: SetMaxEnergy.patch (4k)
Attachment: SetMaxBreath.patch (1023B)
Attachment: FrameBreath.patch (6k)
FrameBreath.patch:> - takebreath = 100 * takebreath / C4MaxPhysical;
C4MaxPhysical is significantly larger than 100, so you're making the filling of breath a lot faster here. I don't know whether that's a good idea, so I just want to make sure that this is intended. Also, an object is an it in English ;-)
SetMaxEnergy.patch:
> - swordman.MaxEnergy = 60000;
> - swordman->DoEnergy(10);
> + swordman->SetMaxEnergy(60000);
SetMaxEnergy won't fill up missing energy, only remove excess energy. That either needs to change, or the DoEnergy call here needs to remain. Hm, doing
NewEnergy = NewMaxEnergy * GetEnergy() / OldMaxEnergy; SetEnergy(NewEnergy); instead of DoEnergy(0) would take care of both problems with SetMaxEnergy :-)
I updated the patches above.
This was already so before, I remove the conversion(DoBreath() and inverse conversion(takebreath), hence it remains the same.
Also in Dutch, no excuses for my lacking linguistic skills ;(
Like this idea, and changed the patche accordingly, though SetEnergy is not available, thus had to use DoEnergy, and line 1370 had to be changed anyways, since effects could change it to zero in any case.
>C4MaxPhysical is significantly larger than 100, so you're making the filling of breath a lot faster here. I don't know whether that's a good idea, so I just want to make sure that this is intended.
This was already so before, I remove the conversion(DoBreath() and inverse conversion(takebreath), hence it remains the same.
>Also, an object is an it in English ;-)
Also in Dutch, no excuses for my lacking linguistic skills ;(
>SetMaxEnergy won't fill up missing energy, only remove excess energy. That either needs to change, or the DoEnergy call here needs to remain. Hm, doing NewEnergy = NewMaxEnergy * GetEnergy() / OldMaxEnergy; SetEnergy(NewEnergy); instead of DoEnergy(0) would take care of both problems with SetMaxEnergy :-)
Like this idea, and changed the patche accordingly, though SetEnergy is not available, thus had to use DoEnergy, and line 1370 had to be changed anyways, since effects could change it to zero in any case.
Good. Do you also want to change the MaxEnergy scale? Changing the scale of the internal Energy value might be too much hassle, but just changing all scripts to not divide MaxEnergy by 1000, and the two uses in the engine to multiply by 1000 should be simple enough.
I'll do that, is there a reason not to do it on the engine side?
Patch is here. For rescaling in the engine we need to consider if changes like DoEnergy(-0.5) are needed, as was possible with the superscaled value. Also energy is still used for structures which can be removed in my opinion, since we have an alternative in C4Script already.
Attachment: RescaleMaxEnergy.patch (6k)
I needed to edit some of the patches a bit to make them apply. I pushed the result to bitbucket again. If nobody tells me of some fatal bug in the patch series, I'll push it to openclonk.org tomorrow.
The second round is almost complete. The only missing thing I know of is MaxEnergy/MaxBreath change notification for the bar displays. I'll probably simply introduce a function to be used to change these, but maybe relying on the Energy/Breath itself changing soon after a maximum change is enough.Useless trivia of the day: The Clonk throws objects with 2.94 px/frame.
The patches broke the slow-walk feature during aiming of bow, using th eshield, striking with the sword etc
Does this patch help?diff --git a/planet/Objects.c4d/Clonk.c4d/Script.c b/planet/Objects.c4d/Clonk.c4d/Script.c
index 3c7865d..b45a022 100644
--- a/planet/Objects.c4d/Clonk.c4d/Script.c
+++ b/planet/Objects.c4d/Clonk.c4d/Script.c
@@ -1239,11 +1239,15 @@ func PushActionSpeed(string action, int n)
if (ActMap == this.Prototype.ActMap)
ActMap = { Prototype = this.Prototype.ActMap };
ActMap[action] = { Prototype = ActMap[action], Speed = n };
+ if (this.Action == ActMap[action].Prototype)
+ this.Action = ActMap[action];
}
/* Resets the named action to the previous one */
func PopActionSpeed(string action, int n) {
// FIXME: This only works if PushActionSpeed and PopActionSpeed are the only functions manipulating the A
+ if (this.Action == ActMap[action])
+ this.Action = ActMap[action].Prototype;
ActMap[action] = ActMap[action].Prototype;
}
Powered by mwForum 2.29.7 © 1999-2015 Markus Wichitill
