ObjectStack extension for card games #218

D8H · 2021-09-14T15:58:48Z

Description

An ordered list of objects and a shuffle action

It provides:

Actions to modify a stack of objects
Conditions to access the objects of a stack
A shuffle action

It can be helpful for:

Card games
Fair randomness (for instance, to create a stack of predetermined bonus and random the order they appear)

Card System Example

Klondike example

Build: https://d8h.itch.io/klondike-solitaire
Project: https://www.dropbox.com/s/0v113sqm2vi9kds/Klondike.zip?dl=1

4ian · 2021-09-14T16:04:02Z

Looks awesome! I've not checked the internals yet, but the video is very impressive. Definitely looks like a great example to add :)
Remind me to also review and merge your Sticky extension if not done by tomorrow evening!

4ian · 2021-09-19T15:37:30Z

Finally got the time to review this! :)

I think the main thing is the usage of gdjs.evtTools.object.pickObjectsIf. I've seen it's used in Contains, ContainsBetween, HasOnTop. This function is usually used to implement free conditions (not behavior conditions) because it will filter the object list maps that are passed as argument.

In the case of your extension, this means that if for example the Contains behavior condition is called for two objects having the behavior (let's imagine we do "Card is in the stack of MyStack", and there are two "MyStack" objects).
The first MyStack Contains method will properly call gdjs.evtTools.object.pickObjectsIf, and filter the objects passed as parameters.
The second MyStack Contains method will also call gdjs.evtTools.object.pickObjectsIf, but the objects will be already filtered.

I don't have a good solution for now - but I have the feeling we're exposing some strange behaviors here if we have more than one "stack object" having the behavior and used at once. Not sure if we can do better, but I can already see confused users asking themselves why only some cards in their stacks are picked by the condition when they have multiple stacks 🤔

Does this make sense?

4ian · 2021-09-19T15:59:07Z

Actually I do have a good solution! Which is to use the API made to filter two list of objects at once:
gdjs.evtTools.object.twoListsTest

This means that the behavior methods that are using pickObjectsIf should be moved to free functions outside the behavior. In these functions, you take a list of objects (as a parameter) with this behavior (as another parameter), and the list of other objects (as another parameter).

You can then call gdjs.evtTools.object.twoListsTest and pass the object lists and write a predicate that verifies that the first object passed as parameter (which will be a stack) contains or not the second one. That should be enough and work in all cases.

Let me know if you give this a try. I can also take a try at this if you wish!

D8H · 2021-09-19T18:43:07Z

Actually I do have a good solution! Which is to use the API made to filter two list of objects at once:
gdjs.evtTools.object.twoListsTest

This means that the behavior methods that are using pickObjectsIf should be moved to free functions outside the behavior. In these functions, you take a list of objects (as a parameter) with this behavior (as another parameter), and the list of other objects (as another parameter).

You can then call gdjs.evtTools.object.twoListsTest and pass the object lists and write a predicate that verifies that the first object passed as parameter (which will be a stack) contains or not the second one. That should be enough and work in all cases.

Let me know if you give this a try. I can also take a try at this if you wish!

Ok, thanks, I'll try this.

D8H · 2021-09-22T17:56:17Z

I committed to save a version before using twoListsTest.

D8H · 2021-09-25T00:16:04Z

I updated the 2 examples to use the new conditions.

4ian · 2021-09-28T21:27:37Z

These new conditions look great!
Last stuff though which is a bit of a problem: the fact that we're storing gdjs.RuntimeObject directly means that we have to handle clearing them from the stack if we destroy one - otherwise we have a "dangling pointer" to one, and the object (and its renderer, and its behaviors, and its variables...) will survive in memory.

I'm not sure what's the best solution, surely to hook again like you did in the other extension to a gdjs callback to listen to object destruction? This might be a pain because you have to inspect all the stacks... or keep a map of "object id to their stack object" - so that whenever an object is deleted, you can quickly get its stack (if any) and remove it?

If this kind of behavior is becoming more common, we could introduce something in GDJS directly, something like a "RuntimeObjectWeakReferenceMap" (sorry for the long name, but you get the idea), that would help doing this for an extension (storing references to objects, and automatically dropping them if the object is deleted).

D8H · 2021-09-28T21:54:17Z

I'm not sure what's the best solution, surely to hook again like you did in the other extension to a gdjs callback to listen to object destruction? This might be a pain because you have to inspect all the stacks... or keep a map of "object id to their stack object" - so that whenever an object is deleted, you can quickly get its stack (if any) and remove it?

An inverted index will allow to make the conditions more efficient anyway. It will also allow to ensure uniqueness too, because I think allowing to add the same instance in one stack twice is probably not a good idea.

arthuro555 · 2021-09-28T21:56:45Z

I wanted to make a semi-related feature request for a long while now actually. A very common pattern with behaviors is to use some of them to "Mark" some objects to be used by another one, similarly to an object group. See for example the LightObstacle, Platform, and Pathfinding obstacle behaviors. Making those with events is possible via the usage of scene variables, but is quite tricky and limited.

To make that easier, my idea is to expose to all functions in extensions for each behavior of the extension a group that references all objects on the scene with that behavior. They would work like normal object groups, except that instead of using runtimeScene.getObjects, it would use another function runtimeScene.getBehaviorObjects(behaviorname: string) that

Get all objects that have the behavior from the game project data
Get them all via runtimeScene.getObjects
Filter then by behavior activation
Return that objects list 😎

The type of that object group would be the same as the Object parameter in the Behavior functions.
This eliminates memory leaks risks as objects list are cleared after each frame anyways, and gives events new powerful features. Not sure if it is applicable to that specific case though.

This would allow for many powerful things:

Being able to use that popular pattern of marking objects to be used by an extension easily (and also store object-specific state via the behavior properties ^^)
Create stuff like the stick extension via events & 2 behaviors Sticker and Stickeable, by linking the objects Sticking to a Stickeable when sticking, then set the position of all Sticking to Stickeable.X() while taking into account linked objects each frame
Allow making behavior that needs access to all objects that have it for updating it, for example, if I want to make an extension that sorts by the value of the object variable weight, I can make an extension wide post events callback that will be able to know the weight of all objects at once instead of using a behavior update method that only knows about its own object instance

4ian · 2021-09-29T16:16:51Z

An inverted index will allow to make the conditions more efficient anyway. It will also allow to ensure uniqueness too, because I think allowing to add the same instance in one stack twice is probably not a good idea.

Sounds good!

I wanted to make a semi-related feature request for a long while now actually.

Let's talk about this in 4ian/GDevelop#3001

My main concern is that naively iterating on behaviors will lead to bad performances as soon as the number of objects grow.

D8H · 2021-10-02T22:00:20Z

Actually, I only used sets because an inverted index would make it a lot more complicated for little value.

A global Set contains every living objects that has been into a stack allows to check in O(1) deleted objects that never been in a stack.
A Set for each stack allows to check deleted objects that once been in a stack in O(number of stacks).

D8H · 2021-10-09T23:00:43Z

Is it a bot issue: fatal: You are not currently on a branch.?

4ian · 2021-10-12T16:34:32Z

This problem should be gone because the registry does not need to be rebuilt and pushed anyway now.
Let's see if we can rebase this on main: !rebase

github-actions · 2021-10-12T16:34:48Z

❗ An internal error has occured. See logs at https://github.com/GDevelopApp/GDevelop-extensions/actions/runs/1334007416.

arthuro555 · 2021-10-12T16:38:37Z

Probably an issue due to the branch to merge from being on a fork, but actions run on the main repository.

See this open issue.

4ian · 2021-10-12T17:07:46Z

scripts/lib/ExtensionsValidatorExceptions.js

+        'RuntimeObject',
+      ],
+      gdjsEvtToolsAllowedProperties: ['object'],
+      runtimeSceneAllowedProperties: ['__allObjectStacks', '__allUsedObjects'],


Sorry I just noticed this now.

__allUsedObjects => this is way way too generic, there is a risk for collision with another extension in the future or user code that would want to use something like this for another purpose.
Instead, let's name it something like __objectStacks_allUsedObjects.
Basically, any name that is on runtimeScene/gdjs is a hack and we should use names as long and precise as possible.
The best solution would be to provide an extension and runtimeScene specific storage for each extension, so that no clash can ever happen.

4ian · 2021-10-12T17:08:32Z

Extensions/ObjectStack.json

+              "disabled": false,
+              "folded": false,
+              "type": "BuiltinCommonInstructions::JsCode",
+              "inlineCode": "const object = objects[0];\nconst behaviorName = eventsFunctionContext.getBehaviorName(\"Behavior\");\nconst behavior = object.getBehavior(behaviorName);\n\nbehavior.objectStack = [];\n// Make contains(), remove() and unicity checks more efficients.\nbehavior.objectSet = new Set();\n\nif (!runtimeScene.__allObjectStacks) {\n  runtimeScene.__allObjectStacks = new Set();\n  // It's only use is to have a O(1) check for\n  // deleted objects that have never been in a stack.\n  runtimeScene.__allUsedObjects = new Set();\n  // Remove from deleted objects from stacks.\n  gdjs.registerObjectDeletedFromSceneCallback(function (runtimeScene, obj) {\n    if (runtimeScene.__allUsedObjects.has(obj)) {\n      runtimeScene.__allUsedObjects.delete(obj);\n      for (const behavior of runtimeScene.__allObjectStacks) {\n        /** @type {gdjs.RuntimeObject[]} */\n        const stack = behavior.objectStack;\n        /** @type {Map<gdjs.RuntimeObject> */\n        const objectSet = behavior.objectSet;\n        if (objectSet.has(obj)) {\n          // There should be only one occurrence, but check the whole array just in case.\n          for (let index = stack.indexOf(obj); index >= 0; index = stack.indexOf(obj, index)) {\n            stack.splice(index, 1);\n          }\n          objectSet.delete(obj);\n        }\n      }\n    }\n  });\n}\nruntimeScene.__allObjectStacks.add(behavior);",


It's only use is to have a O(1) check for => Its only use is to have a O(1) check for

4ian · 2021-10-16T13:14:26Z

Thank you! 👍👍

arthuro555 added ✨ New extension A new extension ⌨ JavaScript Uses JavaScript code, and thereby needs a reviewer who knows JavaScript. labels Sep 14, 2021

D8H mentioned this pull request Sep 26, 2021

[Sticker] Avoid a 1 frame delay when chaining objects. #226

Merged

D8H added 8 commits October 2, 2021 23:11

ObjectStack extension for card games

6bfd155

Remove useless loops

2918842

Remove other useless loops

aab8175

fix a "continue" without any loop.

e4b83a2

Add some functions

f9adf79

Use twoListsTest.

caf1d78

Use sets handle object deletions more efficiently.

7369189

Declare exceptions

49cf004

D8H force-pushed the ObjectStack branch from 8485f92 to 49cf004 Compare October 2, 2021 21:19

Remove unused prototype access.

89819eb

Add a comment.

8595351

Merge branch 'main' into pr/218

f045e20

4ian reviewed Oct 12, 2021

View reviewed changes

4ian approved these changes Oct 12, 2021

View reviewed changes

D8H added 3 commits October 16, 2021 14:45

Review changes: rename and typo

0699408

Update exception

6a34337

Format

6cb098f

4ian merged commit 18d362d into GDevelopApp:main Oct 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ObjectStack extension for card games #218

ObjectStack extension for card games #218

D8H commented Sep 14, 2021 •

edited

Loading

4ian commented Sep 14, 2021

4ian commented Sep 19, 2021

4ian commented Sep 19, 2021

D8H commented Sep 19, 2021

D8H commented Sep 22, 2021

D8H commented Sep 25, 2021

4ian commented Sep 28, 2021

D8H commented Sep 28, 2021

arthuro555 commented Sep 28, 2021 •

edited

Loading

4ian commented Sep 29, 2021

D8H commented Oct 2, 2021 •

edited

Loading

D8H commented Oct 9, 2021

4ian commented Oct 12, 2021

github-actions bot commented Oct 12, 2021

arthuro555 commented Oct 12, 2021 •

edited

Loading

4ian Oct 12, 2021

4ian Oct 12, 2021

4ian commented Oct 16, 2021

ObjectStack extension for card games #218

ObjectStack extension for card games #218

Conversation

D8H commented Sep 14, 2021 • edited Loading

Description

Card System Example

Klondike example

4ian commented Sep 14, 2021

4ian commented Sep 19, 2021

4ian commented Sep 19, 2021

D8H commented Sep 19, 2021

D8H commented Sep 22, 2021

D8H commented Sep 25, 2021

4ian commented Sep 28, 2021

D8H commented Sep 28, 2021

arthuro555 commented Sep 28, 2021 • edited Loading

4ian commented Sep 29, 2021

D8H commented Oct 2, 2021 • edited Loading

D8H commented Oct 9, 2021

4ian commented Oct 12, 2021

github-actions bot commented Oct 12, 2021

arthuro555 commented Oct 12, 2021 • edited Loading

4ian Oct 12, 2021

Choose a reason for hiding this comment

4ian Oct 12, 2021

Choose a reason for hiding this comment

4ian commented Oct 16, 2021

D8H commented Sep 14, 2021 •

edited

Loading

arthuro555 commented Sep 28, 2021 •

edited

Loading

D8H commented Oct 2, 2021 •

edited

Loading

arthuro555 commented Oct 12, 2021 •

edited

Loading