Not logged inOpenClonk Forum
Up Topic Development / Developer's Corner / Modularity of game content
- - By Marky [de] Date 2015-02-02 15:26
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 ;)
Parent - - By Maikel Date 2015-02-02 15:48
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.
Parent - - By Marky [de] Date 2015-02-02 17:27
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:


// 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
Parent - - By Sven2 Date 2015-02-02 18:36
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);
Parent - - By Marky [de] Date 2015-02-02 18:43

>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.
Parent - By Maikel Date 2015-02-02 19:26
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.
Parent - By Sven2 [de] Date 2015-02-02 23:11

> 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.
Parent - - By Pyrit Date 2015-02-11 20:44
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. :/
Parent - - By Maikel Date 2015-02-11 22:23
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.
Parent - - By Pyrit Date 2015-02-12 14:38
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. :/
Parent - By Maikel Date 2015-02-12 14:52
No problem, there are also the basements which needs similar features.
Parent - - By Maikel Date 2015-02-13 12:05
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.
Parent - - By Pyrit Date 2015-02-13 12:15 Edited 2015-02-13 12:17
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.
Parent - By Maikel Date 2015-02-13 12:19
No, problem I will have to tackle many problems before including it and having the library general enough.
Parent - - By Marky [de] Date 2015-07-02 21:07
First suggested addition to our general repository: Vector operations with proplist vectors.
Parent - - By Maikel Date 2015-07-02 21:19
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.
Parent - By Marky [de] Date 2015-07-02 21:48
No idea about speed/performance. Array access has the problem that it is not as readable, you can more easily mess up the indices.

>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.
Parent - - By Sven2 [us] Date 2015-07-02 22:10
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.?
Parent - By Marky [de] Date 2015-07-03 04:26 Edited 2015-07-03 05:02
Yes, arrays are more flexible in this case. I found that 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.
Parent - - By Marky [de] Date 2015-07-05 18:47
Update: As suggested, the same changes, with arrays instead of proplists, as an object, and with more functions.
Parent - - By Zapper [de] Date 2015-07-05 20:44
I'd suggest either putting the functions into a "namespace" as Sven suggested or at l east prefixing them with 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.
Parent - - By Marky [de] Date 2015-07-05 21:00
They are not global, so you would have to call Vector->Multiply([x, y], factor) or something similar.
Parent - By Zapper [de] Date 2015-07-05 21:19
ah, ok!
Up Topic Development / Developer's Corner / Modularity of game content

Powered by mwForum 2.29.7 © 1999-2015 Markus Wichitill