Skip to content

Conversation

@tristanbob
Copy link
Contributor

@tristanbob tristanbob commented Mar 27, 2023

Add an action to move along the rectangular border of an object.

Notes:

  • Counterclockwise runs the tweens in reverse, so they only look the same if it is a symmetrical tween (such as easeInOut). In this example, I use the bounce tween, so when using counterclockwise it starts each side with an awkward bounce.
  • I tried using an object group to apply actions, but they didn't seem to work. I couldn't figure out what was happening so I just repeated the actions once per object. (fixed by renaming behaviors)

@tristanbob tristanbob added the 🔄 Example update An update for an existing example label Mar 27, 2023
@tristanbob tristanbob self-assigned this Mar 27, 2023
@tristanbob tristanbob requested a review from D8H March 27, 2023 16:40
@github-actions
Copy link
Contributor

Preview the game(s) changed or added in this Pull Request


This is an automatic message displaying links to the games in this PR - double check the JSON in case of doubt.

@github-actions
Copy link
Contributor

Images automagically compressed by Calibre's image-actions

Compression reduced images by 54.5%, saving 5.05 KB.

Filename Before After Improvement Visual comparison
examples/rectangular-movement/assets/Saw_Off.png 681 bytes 451 bytes -33.8% View diff
examples/rectangular-movement/assets/Saw_On_1.png 681 bytes 451 bytes -33.8% View diff
examples/rectangular-movement/assets/Saw_On_2.png 704 bytes 444 bytes -36.9% View diff
examples/rectangular-movement/assets/Saw_On_3.png 826 bytes 488 bytes -40.9% View diff
examples/rectangular-movement/assets/Saw_On_4.png 733 bytes 459 bytes -37.4% View diff
examples/rectangular-movement/assets/Saw_On_5.png 668 bytes 446 bytes -33.2% View diff
examples/rectangular-movement/assets/Saw_On_6.png 707 bytes 438 bytes -38.0% View diff
examples/rectangular-movement/assets/Saw_On_7.png 827 bytes 488 bytes -41.0% View diff
examples/rectangular-movement/assets/Saw_On_8.png 760 bytes 468 bytes -38.4% View diff
examples/rectangular-movement/assets/tiled_Summer Tile Water.png 2.82 KB 179 bytes -93.8% View diff

4755 images did not require optimisation.

@D8H
Copy link
Contributor

D8H commented Mar 27, 2023

  • Counterclockwise runs the tweens in reverse, so they only look the same if it is a symmetrical tween (such as easeInOut). In this example, I use the bounce tween, so when using counterclockwise it starts each side with an awkward bounce.

I haven't thought about this. The advantage is that it allows to reverse the movement when a tween shape has started without strange speed change, but It's probably not very common.
We could reverse the tween shape according to the clockwise property value but not when the "toggle direction" action is used. And maybe make 2 toggle actions if it's clearer.

  • I tried using an object group to apply actions, but they didn't seem to work. I couldn't figure out what was happening so I just repeated the actions once per object.

I think it should work exactly the same with objects or groups.
How did you identify which rectangle instance to give to each moving instance. Did you use a variable?

I think this aspect may confuse users. At least when they use the expressions themselves, they can know by experience that they need a for each and a way to find the right instance with a collision check for instance.
Maybe it should be a free action outside of the behavior that does this work?

There is still the issue of rectangles that might overlap 2 moving objects. Maybe the one that has the nearest edge should win?

@tristanbob
Copy link
Contributor Author

I haven't thought about this. The advantage is that it allows to reverse the movement when a tween shape has started without strange speed change, but It's probably not very common. We could reverse the tween shape according to the clockwise property value but not when the "toggle direction" action is used. And maybe make 2 toggle actions if it's clearer.

I don't think we need to change anything, it just surprised me. I do understand what you mean about the toggle direction action; it could make the object teleport if it was using an asymmetrical easing.

I think it should work exactly the same with objects or groups.

I fixed this by making the original ship (and all related expressions) use the new behavior name.

I think this aspect may confuse users. At least when they use the expressions themselves, they can know by experience that they need a for each and a way to find the right instance with a collision check for instance. Maybe it should be a free action outside of the behavior that does this work?

I created a matching standalone function, but it doesn't seem to work. Can you review it?

There is still the issue of rectangles that might overlap 2 moving objects. Maybe the one that has the nearest edge should win?

Are you talking about how the creator will match each moving object with the correct center object? Great question!

My first guess is that they could do a collision check (and perhaps create an object link at the start of the scene, instead of constant collision checks.) Do you think we should help them with this? (I think we should. Here is my plan:)

  1. Pick the closest MovingObject to the AABB center of the CenterObject (once)
  2. Create links between the two objects (once)
  3. Use a ForEach() to update the extension properties based on the positions of the linked center objects

- LinkNearestObjects
- Added ForEach and object linking to Move Along function.... which still does not work right :(
@tristanbob
Copy link
Contributor Author

@D8H I tried making a stand-alone function but it is not working (the movement shape doesn't get changed to match the blue object). Can you look at it when you have time?

@D8H
Copy link
Contributor

D8H commented Mar 28, 2023

1. Pick the closest MovingObject to the AABB center of the CenterObject (once)

The center is not the best criteria as moving objects are put on the border. The distance between the border and the moving object should give better results.

This should also allow to set the right starting position.

@tristanbob
Copy link
Contributor Author

How do we find the distance to the border? The border is not a single position.

Also, if patrol patterns (center object) overlap, it's hard to know which moving object goes with each one, unless the user places the moving object over the center point (similar to how joint connector works).

@D8H
Copy link
Contributor

D8H commented Mar 28, 2023

How do we find the distance to the border? The border is not a single position.

By doing a projection. A rectangle is convex so there will only be one result.

The problem can be split in 2 steps:

  • projecting points that are outside
  • and the ones inside

Points that are outside can be projected on the rectangle by doing a clamp on the 2 coordinates. It doesn't affect points that are inside so it can be applied without checking first.

Every points are now inside the rectangle so no need to check either.

Points that are inside must be projected on the nearest edges. The nearest edge distance is the minimum of the 2 delta of X and the 2 delta of Y. Each of the 4 delta can be checked to be the minimum and the points goes to that edge.

- Object linking now loops into both object lists to cover situations where objects overlap.
- Removed original "move object around another object" function
@tristanbob
Copy link
Contributor Author

@D8H I think I found an elegant solution that is both easy to use and powerful.

There is a single action that links each moving object to a center object, but only when the center point of the moving objects is inside the center object.

This enables multiple moving objects to be placed on the center object, and assigned to a specific starting corner (thanks to your great design!)

Screenshot of the event sheet

image

Video

GDevelop_octe4IdSZG.mp4

@D8H
Copy link
Contributor

D8H commented Apr 27, 2023

There is a single action that links each moving object to a center object, but only when the center point of the moving objects is inside the center object.

What if one rectangle is inside another rectangle? (I think this is why distance from the rectangle edges is needed)

@tristanbob
Copy link
Contributor Author

@D8H I did it! Can this implementation be optimized?

GDevelop_iRg5lj138U.mp4

@D8H
Copy link
Contributor

D8H commented Apr 28, 2023

From a quick glance:

  • DistanceToClosestEdgeX is not prefixed with the extension name.
  • Is DistanceToClosestEdgeX never reset?
  • Using 0 as undefined value for DistanceToClosestEdgeX can cause issue because a distance of 0 is a valid value.
  • DistanceToClosestEdgeX could be initialized to a big value as it's used to find a minimum.
  • The conditions on the min search loop is probably useless as the min operation already compare the values.

@D8H
Copy link
Contributor

D8H commented May 5, 2023

I was able to remove conditions from two of the events that generate the X and Y distances of the current CenterObject, however, I need to keep the conditions in the third event because it needs to know if this CenterObject is closer than the saved variable.

image

  • The condition is the same as doing a min with the variable.
  • It avoids to duplicate the other min (in the condition + action).
  • If the 2 previous events are just intermediate local calculus, they can be in the last event directly and even be a private expression.

@tristanbob
Copy link
Contributor Author

  • The condition is the same as doing a min with the variable.
  • It avoids to duplicate the other min (in the condition + action).

I got this working as you described.

  • If the 2 previous events are just intermediate local calculus, they can be in the last event directly and even be a private expression.

I have created 3 new private expressions functions for this. It does really simplify the extension, and those functions could be copied (manually) if other people want to use them.

My only question is what names should I give the parameters? Currently, I'm using "CenterObject" and "MovingObject", but the functions are called "DistanceToClosestEdge" so the name doesn't fit well. Any ideas for these names?

@tristanbob
Copy link
Contributor Author

(Note: I'm repeating this question so it doesn't get missed)

My only question is what names should I give the parameters? Currently, I'm using CenterObject and MovingObject, but the functions are called DistanceToClosestEdge so the name doesn't fit well. Any ideas for these names?

@D8H
Copy link
Contributor

D8H commented May 10, 2023

(Note: I'm repeating this question so it doesn't get missed)

My only question is what names should I give the parameters? Currently, I'm using CenterObject and MovingObject, but the functions are called DistanceToClosestEdge so the name doesn't fit well. Any ideas for these names?

For private functions, keeping the same object name as the caller can be a good option. It's easier to copy paste events from one function to another and if the object name is not too much confusing, it lets code maintainers know it's the same object.

@tristanbob
Copy link
Contributor Author

Ok, that makes sense.

Did you want any other changes on this PR before approving it?

@D8H
Copy link
Contributor

D8H commented May 13, 2023

In the "Update rectangular movement to follow the border of object" group, the foreach on MovingObject is not necessary because the right MovingObject is picked by the LinkedObject action. The extra loop result to the work same work being repeated for each each MovingObject.

The calculus doesn't handle some cases correctly (it chooses the big rectangle in the following screenshot). Please take a look at this previous message: #503 (comment)

image

@github-actions
Copy link
Contributor

Images automagically compressed by Calibre's image-actions

Compression reduced images by 11.4%, saving 46 bytes.

Filename Before After Improvement Visual comparison
examples/rectangular-movement/assets/NewSprite.png 202 bytes 179 bytes -11.4% View diff
examples/rectangular-movement/assets/NewSprite2.png 203 bytes 180 bytes -11.3% View diff

4994 images did not require optimisation.

@tristanbob
Copy link
Contributor Author

In the "Update rectangular movement to follow the border of object" group, the foreach on MovingObject is not necessary because the right MovingObject is picked by the LinkedObject action. The extra loop result to the work same work being repeated for each each MovingObject.

Nice catch, I agree that the linked condition selects the correct Moving object so no loop is necessary. I have removed the loop.

The calculus doesn't handle some cases correctly (it chooses the big rectangle in the following screenshot). Please take a look at this previous message: #503 (comment)

image

Ah, I do see the problem. In your example, the Y value is VERY close to the top edge of the object on the right.
After thinking about this for several hours (and asking ChatGPT) I was able to implement the solution you described.
Please test, and feel free to try the temp scene I made for testing this.

@D8H Thanks for taking the time to work on this!

@tristanbob
Copy link
Contributor Author

tristanbob commented May 18, 2023

Current status:

  • All review suggestions have been addressed successfully
  • Temp new scene added for testing this new action (it will be deleted after review)
  • Waiting for review/approval

@D8H
Copy link
Contributor

D8H commented May 20, 2023

What do you think about creating a dedicated example to explain the new action?

  • It will allow to put more objects in the scene to better show what are the possibilities
  • There will be fewer events so comments won't be missed

It could be something like: "Circular saw platformer" (or a better name). It could be a tiny level with saws on a moving platform.

I think it could be useful to use the "teleport in a corner" action when the objects are linked to put the moving object at the nearest corner.

@tristanbob
Copy link
Contributor Author

That is a good idea. Do you prefer a new scene on this example, or a new example?

@D8H
Copy link
Contributor

D8H commented May 20, 2023

I think a new example will be better because I can see people searching for this in particular.

@tristanbob
Copy link
Contributor Author

I wonder if there was anything I should change in this PR (or just close it)

I know I added a change direction button, but I don't see a commit just for that change.

@D8H
Copy link
Contributor

D8H commented May 20, 2023

I know I added a change direction button, but I don't see a commit just for that change.

It was already here. I think you added labels under the buttons, but I don't think it's necessary.

@tristanbob
Copy link
Contributor Author

@D8H I have cleaned up this PR so that it only includes these changes:

  • Added text descriptions to controls
  • Reverse direction button flips when clicked

I'll start the new PR for something like the saws that move around objects.

@tristanbob tristanbob changed the title [Rectangle Movement] Add an action to move along the rectangular border of an object [Rectangle Movement] Minor updates to add labels to controls Jun 6, 2023
@tristanbob tristanbob merged commit 9a1d253 into main Jun 9, 2023
@tristanbob tristanbob deleted the retangle-movement-move-around-object branch June 9, 2023 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔄 Example update An update for an existing example

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants