Shortly I am going to merge a "bugfix" that changes the behaviour of CreateObject(), as discussed in http://bugs.openclonk.org/view.php?id=1214. The change has the following effects:
CreateObject(id, x, y) creates the object and sets its position to the specified coordinates (which is the same as the old CreateObject(id, x, y)->SetPosition(x, y)).
CreateObjectAbove(id, x, y) behaves like the old CreateObject() and positions the bottom center of the object at the specified coordinates.
For the change I have replaced all CreateObject() calls with CreateObjectAbove() calls, keep this in mind when merging from and to other branches. The merge process can become a little chaotic after the change, so the best option in my opinion is: make sure that the scripts in a branch other than master work correctly with the new behaviour. Then, when someone merges to master we can assume that the calls to CreateObject() are OK and do not have to be replaced with CreateObjectAbove() again.
Some scripts call CreateObjectAbove() and then SetPosition() on the created object, for example when creating brick edges. This cannot simply be replaced with CreateObject(), it causes the edges to disappear. This may hold for other objects as well.
I will update the post once I have committed.
[Update]
I commited a while ago already.
CreateObject(id, x, y) creates the object and sets its position to the specified coordinates (which is the same as the old CreateObject(id, x, y)->SetPosition(x, y)).
CreateObjectAbove(id, x, y) behaves like the old CreateObject() and positions the bottom center of the object at the specified coordinates.
For the change I have replaced all CreateObject() calls with CreateObjectAbove() calls, keep this in mind when merging from and to other branches. The merge process can become a little chaotic after the change, so the best option in my opinion is: make sure that the scripts in a branch other than master work correctly with the new behaviour. Then, when someone merges to master we can assume that the calls to CreateObject() are OK and do not have to be replaced with CreateObjectAbove() again.
Some scripts call CreateObjectAbove() and then SetPosition() on the created object, for example when creating brick edges. This cannot simply be replaced with CreateObject(), it causes the edges to disappear. This may hold for other objects as well.
I will update the post once I have committed.
[Update]
I commited a while ago already.
I don't like how you implemented this as FnCreateObject->SetPosition(). The result is that the object is still at the wrong coordinates during its callbacks to Construction and Initialize and is then moved. This is bad in case the object creates other stuff during its construction.
This is why I asked twice for someone to look at the code, because I also thought that it is not good and wrote a comment.
The Construction()-call is still correct, but Initialize() will have some problems. I see two options here: In both I add a bool parameter to C4Game::CreateObject, then
a) I pass it all the way down to C4Object::DoCon() so that it does not change y-position when groing
b) manipulate it's position after the Construction()-call so that it will end up in the desired position when Initialize() is called.
The Construction()-call is still correct, but Initialize() will have some problems. I see two options here: In both I add a bool parameter to C4Game::CreateObject, then
a) I pass it all the way down to C4Object::DoCon() so that it does not change y-position when groing
b) manipulate it's position after the Construction()-call so that it will end up in the desired position when Initialize() is called.
I'd say just pass a parameter to C4Game::CreateObject, create the object at 100% con and then call Initialize directly from C4Game::CreateObject. This avoids any object movement from DoCon and also lots of duplicate shape updates, etc.
This is not as trivial as it sounds. You can view my version on gitHub. Skipping the calls in DoCon() leads to assertion, also the object does not have all components (those problems should be fixed in my branch) and the vertices are not where they should be. It seems easier to add the parameter to DoCon().
You would have to call some of the update functions from DoCon (like UpdateShape, UpdateFace, etc.). But if you think the extra parameter is easier then go ahead and use that one. Maybe the flag could even be made available to script in FnDoCon then?
The updated version is in my repository. Things left to do: documentation (I have to add the missing parameters to the docu or change the parameter order, because precision comes after change).
A minor detail is the parameter order of NewObject or CreateObject, I placed the boolean flag after the rotation parameter. Also, the parameter name has to be improved imo.
A minor detail is the parameter order of NewObject or CreateObject, I placed the boolean flag after the rotation parameter. Also, the parameter name has to be improved imo.
Looks good; I don't see anything obvious. I find the parameter "offset_stays" not very descriptive because "offset" in the context of C4Object is already used by the distance of the top left corner of the graphics from the object position. Maybe "position_stays", "fixed_position" or "grow_from_center". Or reverse it and make it "!fixed_bottom" or "!grow_ from_bottom".
Where can I find these changes?
I used CreateObject on master and my objects were invisible (may depend on the script of the object), and they were not with CreateObjectAbove. Will you test CreateObject extensively?
I used CreateObject on master and my objects were invisible (may depend on the script of the object), and they were not with CreateObjectAbove. Will you test CreateObject extensively?
>I pass it all the way down to C4Object::DoCon() so that it does not change y-position when groing
Oi, are you about to fix the shaking when objects are built? (And trees growing, just much slower) :>
The lowest vertex stays in place, but it seems to toggle the distance of vertex to offset when growing, so the whole object appears to be shaking. There should be some way around that. This appeared in Clonk Rage already, but it was not noticeable as much, because it toggles only by 1 pixel and you don't have zoom.
Powered by mwForum 2.29.7 © 1999-2015 Markus Wichitill