I am developping some libraries and stuff for OC since the beginning of this year and the following things occurred to me:
- for different projects I tend to copy a .c-file with constants that should be available from the engine from one project to another
- the number 36 occurrs very often. Replacing it by an appropriate constant would make the code more understandable for new coders. The same holds for other hardcoded numbers, such as "25" in Clonk_Animations: It occurrs in at least 3 functions, so if I wanted to change the default rotation of the Clonk for whatever reason I would have to overload every one of these functions.
- some functions do a lot of stuff, but I want to change/remove only a part of that behaviour, so I have to overload the whole function which is not optimal imo.
I wanted to create a "Modularity" branch either in the general repository or in my private repository, so that I can push my changes somewhere. So far I have not changed anything in the original objects, but I stumble upon situations like the above again and again, so I want to change something. Before I am doing this though, I ask for opinions on the forum, because if I base my projects on a branch that nobody wants on master, then I have to do double the work in the end ;)
- for different projects I tend to copy a .c-file with constants that should be available from the engine from one project to another
- the number 36 occurrs very often. Replacing it by an appropriate constant would make the code more understandable for new coders. The same holds for other hardcoded numbers, such as "25" in Clonk_Animations: It occurrs in at least 3 functions, so if I wanted to change the default rotation of the Clonk for whatever reason I would have to overload every one of these functions.
- some functions do a lot of stuff, but I want to change/remove only a part of that behaviour, so I have to overload the whole function which is not optimal imo.
I wanted to create a "Modularity" branch either in the general repository or in my private repository, so that I can push my changes somewhere. So far I have not changed anything in the original objects, but I stumble upon situations like the above again and again, so I want to change something. Before I am doing this though, I ask for opinions on the forum, because if I base my projects on a branch that nobody wants on master, then I have to do double the work in the end ;)
Well for the 36 I agree, we should name that FPS since that is literally the conversion from frames to second. But Ctrl+F for 36 won't do the job there, 36 is also 35, 37 and 38 a lot of times.
I have no idea what 25 stands for. Also can you give a concrete example of which function you want to modify in what way?
I think it is easiest to develop this on a github branch for now, since there it is easy to just rebase the branch on top of master continuously.
I have no idea what 25 stands for. Also can you give a concrete example of which function you want to modify in what way?
I think it is easiest to develop this on a github branch for now, since there it is easy to just rebase the branch on top of master continuously.
I have several examples:
I want the CTF goal flag to not slow down the player and I want to remove the particle trail. Removing the slowdown is very easy, because I just overwrite ReducePhysicals() and ResetPhysicals() with empty functions. For the particles I have to overwrite FxFlagCarriedTimer(). My proposed change would be:
Why did I add functions that find a base and functions that drop the flag? In case someone has a different base object or if he wants to implement different effects when the flag is dropped.
For another project I wanted the list of sellable contents to also contain stuff that is in the Clonk and any objects he is grabbing. So I had to overload a lot stuff in Library_Base:
* all calls UpdateSellList() to UpdateSellList(pClonk)
* UpdateSellList: GetSellableContents() => GetSellableContents(pClonk)
* DoSell: Remove some lines such as the object entering the base on selling, and adding some stuff for the new behaviour.
=> in this case I would not change the base functions
Also I wanted object interactions to be displayed by the GUI_Controller even if the Clonk is contained in a vehicle or structure. For my overload it would have been useful if the lines marked by comments in FxIntSearchInteractionObjectsTimer() were separate functions:
// add extra-interactions Priority 0
// add interactables (script interface)
// add extra-interactions Priority 1
// add extra-interactions Priority 2
I want the CTF goal flag to not slow down the player and I want to remove the particle trail. Removing the slowdown is very easy, because I just overwrite ReducePhysicals() and ResetPhysicals() with empty functions. For the particles I have to overwrite FxFlagCarriedTimer(). My proposed change would be:
// Checks whether the carrier has reached its base.
protected func FxFlagCarriedTimer(object target, effect)
{
var controller = target->GetController();
var ctrl_team = GetPlayerTeam(controller);
var x = effect.x;
var y = effect.y;
var newx = target->GetX();
var newy = target->GetY();
// Draw partical line following the flag.
if (Distance(x, y, newx, newy) > 5)
{
target->CreateParticle("SphereSpark", 0, 0, 0, 0, PV_Random(36 * 3, 36 * 3 + 10), effect.tracer_particles);
effect.x=newx;
effect.y=newy;
}
// Draw partical line following the flag.
FxFlagCarriedParticleEffects(target, effect);
// Search for nearby base to drop flag and score a point.
var base = FindObject(Find_ID(Goal_FlagBase), Find_Func("FindTeam", ctrl_team), Find_Distance(20));
if (base && base->IsBaseWithFlag())
{
var goal = FindObject(Find_ID(Goal_CaptureTheFlag));
if (goal)
goal->AddTeamScore(ctrl_team);
Log("$MsgFlagCaptured$", GetTaggedTeamName(ctrl_team));
BeamFlag(false);
return -1;
}
return 1;
if (DropFlagAtBase(target, effect))
{
return FX_Execute_Kill;
}
else
{
return FX_OK;
}
}
protected func FxFlagCarriedParticleEffects(object target, effect)
{
var x = effect.x;
var y = effect.y;
var newx = target->GetX();
var newy = target->GetY();
// Draw partical line following the flag.
if (Distance(x, y, newx, newy) > 5)
{
target->CreateParticle("SphereSpark", 0, 0, 0, 0, PV_Random(36 * 3, 36 * 3 + 10), effect.tracer_particles);
effect.x=newx;
effect.y=newy;
}
}
protected func DropFlagAtBase(object target, effect)
{
var controller = target->GetController();
var ctrl_team = GetPlayerTeam(controller);
var base = FindBaseToDropAt(ctrl_team);
if (base)
{
OnDropFlagAtBase(ctrl_team);
return true;
}
return false;
}
protected func FindBaseToDropAt(int ctrl_team)
{
return FindObject(Find_ID(Goal_FlagBase), Find_Func("FindTeam", ctrl_team), Find_Func("IsBaseWithFlag"), Find_Distance(20));
}
protected func OnDropFlagAtBase(int ctrl_team)
{
var goal = FindObject(Find_ID(Goal_CaptureTheFlag));
if (goal)
goal->AddTeamScore(ctrl_team);
Log("$MsgFlagCaptured$", GetTaggedTeamName(ctrl_team));
BeamFlag(false);
}
Why did I add functions that find a base and functions that drop the flag? In case someone has a different base object or if he wants to implement different effects when the flag is dropped.
For another project I wanted the list of sellable contents to also contain stuff that is in the Clonk and any objects he is grabbing. So I had to overload a lot stuff in Library_Base:
* all calls UpdateSellList() to UpdateSellList(pClonk)
* UpdateSellList: GetSellableContents() => GetSellableContents(pClonk)
* DoSell: Remove some lines such as the object entering the base on selling, and adding some stuff for the new behaviour.
=> in this case I would not change the base functions
Also I wanted object interactions to be displayed by the GUI_Controller even if the Clonk is contained in a vehicle or structure. For my overload it would have been useful if the lines marked by comments in FxIntSearchInteractionObjectsTimer() were separate functions:
// add extra-interactions Priority 0
// add interactables (script interface)
// add extra-interactions Priority 1
// add extra-interactions Priority 2
Overloading internal functions is usually dangerous. If someone ever rewrites the code, they may rename the effects and functions and your pack will no longer work.
So, for functions that are supposed to be overwritten, comments should be added that they are now part of the interface (like "// may be overwritten by derived objects").
For the particle trail and the slowdown, I'd rather suggest the CTF goal should make these things configurable. Like goal->SetFlagSlowdown(amount); goal->SetFlagTrail(opt);
So, for functions that are supposed to be overwritten, comments should be added that they are now part of the interface (like "// may be overwritten by derived objects").
For the particle trail and the slowdown, I'd rather suggest the CTF goal should make these things configurable. Like goal->SetFlagSlowdown(amount); goal->SetFlagTrail(opt);
>Overloading internal functions is usually dangerous. If someone ever rewrites the code, they may rename the effects and functions and your pack will no longer work.
Which is exactly why I want stuff to be more modular. Also I do not want to duplicate objects entirely just to change 2 or 3 lines of code.
Yes, and that is good, you can post your patches here and if nobody replies to them within a week you can push them to the main repos.
> Which is exactly why I want stuff to be more modular. Also I do not want to duplicate objects entirely just to change 2 or 3 lines of code.
I had the impression that you did want to outsource functionality into subfunctions and then overload those subfunctions (e.g.: overload "FxFlagCarriedParticleEffects" with an empty function). That is fine of course and it would be cool if you could do some cleanup :-) But should make clear in the original script that the names of these functions are part of the interface and should not be changed if possible.
And for stuff that is likely to be overloaded - e.g. the slowdown effect - I'd rather have it controlled by settings because it's likely that scenarios want to configure this anyway for balancing. It can still also go into subfunctions of course.
Speaking of modularity:
The hammer is super undynamic... At least when you try to build bridge segments...
It is necessary to specify a bigger build radius. Half the structure's width and height is not enough. (You can also notice that on the windmill.)
The preview should not stick to the floor, but float with the cursor.
The preview should turn green on self defined rules. Because the bridge only turns green when one edge is sticking in material.
The creation of the construction site must have it's own rules. Because the bridge is floating partly in mid air, the normal Checkconstruction site doesn't work. Also the construction site must be bigger then it's preview so that you can put materials into it when you are standing on the edge.
I had that all done in clonkomotive but I wrote tons of specific ifs and checks into the hammer itself to make it work. I did a second try on making the buildings itself define all these criteria but that ended up as a mess and never worked. I guess it would be required to rewrite the whole hammer to make that stuff work. :/
The hammer is super undynamic... At least when you try to build bridge segments...
It is necessary to specify a bigger build radius. Half the structure's width and height is not enough. (You can also notice that on the windmill.)
The preview should not stick to the floor, but float with the cursor.
The preview should turn green on self defined rules. Because the bridge only turns green when one edge is sticking in material.
The creation of the construction site must have it's own rules. Because the bridge is floating partly in mid air, the normal Checkconstruction site doesn't work. Also the construction site must be bigger then it's preview so that you can put materials into it when you are standing on the edge.
I had that all done in clonkomotive but I wrote tons of specific ifs and checks into the hammer itself to make it work. I did a second try on making the buildings itself define all these criteria but that ended up as a mess and never worked. I guess it would be required to rewrite the whole hammer to make that stuff work. :/
Since I am cleaning up the libraries, and the hammer is based on a library maybe I can have a look some time soonish. Maybe you can attach a copy of the bridge (or the latest version of Clonkomotive) here and I can have a look.
The only good version would be the one that's already uploaded. The current one is pretty much an empty script file, because I didn't know where to start and I didn't save the other tries, because they were so bad, sorry. :/
No problem, there are also the basements which needs similar features.
Is it okay if I move the wooden bridge into the repository? It might be nice for other scenarios as well, e.g. Crash.ocs.
Sure, go ahead! :)
But it might not be 100% bugfree, last time it had some problems with the construction site snapping when a segment got destroyed.
But it might not be 100% bugfree, last time it had some problems with the construction site snapping when a segment got destroyed.
First suggested addition to our general repository: Vector operations with proplist vectors.
Three comments:
* What about the speed? And what would you use it for? Is there a reason for not doing this with arrays?
* Why not Vector(int x, int y, int z, int u, ...) instead of Vector_2D and Vector_3D?
* I did not see an outter product implemented.
* What about the speed? And what would you use it for? Is there a reason for not doing this with arrays?
* Why not Vector(int x, int y, int z, int u, ...) instead of Vector_2D and Vector_3D?
* I did not see an outter product implemented.
No idea about speed/performance. Array access has the problem that it is not as readable, you can more easily mess up the indices.
Is there a possibility to have a variable amount of parameters? Otherwise the largest supported dimension would be 10 for that standard 'constructor'.
Randrian uses these vectors, in the array version, in his rope library and I need something similar for one of my objects. The landscape normal thing already uses a proplist vector of this kind. So it seems to have some use.
>Why not Vector(int x, int y, int z, int u, ...) instead of Vector_2D and Vector_3D
Is there a possibility to have a variable amount of parameters? Otherwise the largest supported dimension would be 10 for that standard 'constructor'.
Randrian uses these vectors, in the array version, in his rope library and I need something similar for one of my objects. The landscape normal thing already uses a proplist vector of this kind. So it seems to have some use.
I agree with Maikel that arrays seem more natural to me for this kind of math.
In either case, instead of polluting the global namespace, what about just naming the library "Vec" and then calling them as Vec->Add, etc.?
In either case, instead of polluting the global namespace, what about just naming the library "Vec" and then calling them as Vec->Add, etc.?
Yes, arrays are more flexible in this case. I found that
Why does System.ocg/Math.c/GetSurfaceVector(int x, int y) return a proplist {x, y} then? This one sticks out oddly and was the reason for my to switch to proplist, because apparently someone else was using this format already.
The library is a good idea!
Edit: The proplist implementation shows its weakness once you think about matrix operations.
SetPosition(vec1.x, vec1.y + vec2.y)
is more readable than SetPosition(vec1[0], vec1[1] + vec2[1])
. Changing the implementation back to arrays (I actually used arrays before I switched to proplists) is easy enough.Why does System.ocg/Math.c/GetSurfaceVector(int x, int y) return a proplist {x, y} then? This one sticks out oddly and was the reason for my to switch to proplist, because apparently someone else was using this format already.
The library is a good idea!
Edit: The proplist implementation shows its weakness once you think about matrix operations.
Update: As suggested, the same changes, with arrays instead of proplists, as an object, and with more functions.
I'd suggest either putting the functions into a "namespace" as Sven suggested or at l east prefixing them with
Also, I would rename "
Vec
for example. For me it would f.e. not clear why Multiply(5, 2)
wouldn't work.Also, I would rename "
Length
" to "Norm
" or something, because we already have GetLength
.
Powered by mwForum 2.29.7 © 1999-2015 Markus Wichitill