Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ai): Add Pursue behavior from gdx-ai #1267

Closed
wants to merge 34 commits into from

Conversation

st-pasha
Copy link
Contributor

@st-pasha st-pasha commented Dec 23, 2021

Description

This is the beginning of the work on porting the gdxAI library into dart and integrating it into the Flame engine.

This PR adds the Pursue behavior, and implements some of the basic interface classes such as Steerable, Limiter, Location, SteeringBehavior, and SteeringAcceleration.

I have also made some modifications to the original Pursue algorithm, for comparison the timings of the old and the new algorithm are as follows:

test name original time current time
pursuit exact 7.0 7.0
pursuit 1 4.34 4.28
pursuit 2 1000 14.34
pursuit 3 1000 2.6
pursuit 4 12.90 4.8
pursuit 5 20.26 10.84
pursuit 6 1.18 1.18
chase 1 305.6 302.8
chase 2 252.6 250.4
chase 3 17.3 16.2
Screen.Recording.2021-12-23.at.3.06.25.PM.mov

Checklist

  • The title of my PR starts with a Conventional Commit prefix (fix:, feat:, docs: etc).
  • I have read the Contributor Guide and followed the process outlined for submitting PRs.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples.

Breaking Change

  • No, this is not a breaking change.

Related Issues

WIP for #1164

@spydon
Copy link
Member

spydon commented Dec 23, 2021

Maybe this should be flame_ai bridge package? Do we need to include the licence in the header of each file, can it not be in its separate licence file?

@st-pasha
Copy link
Contributor Author

st-pasha commented Dec 23, 2021

Maybe this should be flame_ai bridge package?

Well, there is nothing to bridge to. The gdx-ai library is in Java, so cannot be referenced directly. Plus, this library will eventually be deeply integrated with main Flame; for example all logic for camera motion are the examples of behaviors (seek/pursuit), so we'll refactor them later to make Camera steerable.

Do we need to include the licence in the header of each file, can it not be in its separate licence file?

Here's how I understand the conditions imposed upon us by the Apache-2 license:

  1. "You must give any other recipients of the Work or Derivative Works a copy of this License;" -- this copy is in the readme file;
  2. "You must cause any modified files to carry prominent notices stating that You changed the files;" -- this is why every file has "translated into Dart with modifications" in capital letters;
  3. "You must retain, in the Source form of any Derivative Works that You distribute, all copyright, patent, trademark, and attribution notices from the Source form of the Work, excluding those notices that do not pertain to any part of the Derivative Works" -- this is why every file carries the copy of the boilerplate note taken from the original file;
  4. "If the Work includes a "NOTICE" text file as part of its distribution ..." -- not applicable since they don't have a NOTICE file.

I've also included a line stating that the translated file's copyright really belongs to the Blue Fire project, not to the original gdxAI team. This is explicitly allowed by the Apache-2 license:

"You may add Your own copyright statement to Your modifications and may provide additional or different license terms and conditions for use, reproduction, or distribution of Your modifications, or for any such Derivative Works as a whole, provided Your use, reproduction, and distribution of the Work otherwise complies with the conditions stated in this License."

@st-pasha st-pasha marked this pull request as ready for review December 24, 2021 21:29
@luanpotter
Copy link
Member

Porting an AI library should be done in a bridge package, like flame_gdx_ai. Similar to how we do flame_box/forge2d.
If our core Flame code is not flexible enough to allow any couplings we want to be done from the flame_x package side, we should, instead, improve it so it does.

@st-pasha
Copy link
Contributor Author

st-pasha commented Dec 26, 2021

Porting an AI library should be done in a bridge package, like flame_gdx_ai. Similar to how we do flame_box/forge2d.

There is forge2d package, and flame_forge2d serves as a bridge, allowing to easier use forge2d library together with the core Flame. Similarly, there is rive package, and then flame_rive serves as a bridge to bring Rive animations into Flame. But there is no gdx-ai package (not in Dart ecosystem at least). There is nothing to bridge to.

In addition, this functionality does belong to the Flame core. We already have some rudimentary steering behaviors, although they are done in a bespoke non-reusable way. So, this is going to be a massive upgrade of the existing functionality (I mean, not this PR, but this project as a whole).

@luanpotter
Copy link
Member

Despite the name, the bridge packages don't necessarily need to "bridge" to anything external, even though most do. For example, flame_splash_screen doesn't bridge to a 3rd package; rather, it contains within itself all of its functionality, that we thought to separate from Flame just for separation of concerns. The forge2d package exists because we decided to created it, as a place to port box2d (existing non-dart lib) into dart. Similarly, if we are porting an external library from another language, it would be much more valuable to encapsulate it in a dart package, so that anyone can use it, even if they don't use Flame itself, or even Flutter. Then, the flame bridge package would just bridge that into Flame components and reusable pieces. Similar to how we extracted canvas_test and that package is valuable to anyone testing canvas, idependently of Flame. Regardless of the 3rd package being made (to provide the implementation to the dart community outside of Flame), I think the code should be better separated into its own package. I am not aware of any AI related coded currently inside Flame, sounds like a very extensive set of tools that would be great to have but feel like could be properly separated. I am not sure what you mean by steering behaviors.

@st-pasha
Copy link
Contributor Author

Separation of concerns is great when done on the level of methods, or classes, or even sub-packages. But separating things into different libraries is a step too far. Just because effects can, in theory, be torn away and put into a separate library -- doesn't mean they should. Just because particle systems, or collision detection, or some other components like text or parallax can be separated into their own tiny libraries -- doesn't mean they should.

There is very little in terms of benefits that can be achieved there, whereas the cost is quite immediate and real. These costs are: (1) the need to maintain another 2 separate libraries, (2) the need to build a separate documentation system, (3) the need to create a new test harness, (4) the need to build a separate example library, (5) the cost of synchronizing APIs between libraries, (6) separate release schedule, etc.

Taken together, that's a big cost. And I'm not saying this theoretically: from the very practical standpoint, none of the libraries that we released as separate packages achieve the same level of quality as the main Flame library.

In short, I'm not in favor of shooting a project in the foot before it even had a chance to make its first baby steps.

@spydon
Copy link
Member

spydon commented Dec 28, 2021

I agree with this, we shouldn't make bridge libraries just because we can, it will be less easy for the user to use too. But I think we need a plan for what should be ported here, if there will be a lot of changes I think this should start off as a separate package (not two) with a version <1.0.0 and then simply move it in when it is done. If this feat is expected to have a fairly stable api within 2 months (next stable release) then I think it can be kept in the main package.

@st-pasha
Copy link
Contributor Author

It is difficult to anticipate the amount of change that will be needed. On one hand, the source library is quite stable. On the other, there's been quite a few non-trivial adjustments to their code just in making this PR.

We could simply avoid exporting this into main Flame namespace. Thus, if someone wants to use it, they'd have to access it via package:flame/lib/src/ai/....

@erickzanardo
Copy link
Member

I agree with people that this should be a separate package.

@spydon
Copy link
Member

spydon commented Dec 30, 2021

We could simply avoid exporting this into main Flame namespace. Thus, if someone wants to use it, they'd have to access it via package:flame/lib/src/ai/....

I think that sounds like a pretty good compromise, until it becomes more stable and we'll export it.

Possibly rename the directory to something else than AI, I think this is much more generic than that?

@st-pasha
Copy link
Contributor Author

Possibly rename the directory to something else than AI, I think this is much more generic than that?

What do you have in mind?

To me, "AI" sounds generic enough: artificial intelligence -- any functionality that helps make your game more smart. In addition to behaviors, we'll also have reactive actions, movement in formations, path-finding, behavior trees, etc.

@spydon
Copy link
Member

spydon commented Dec 30, 2021

We talked in the team yesterday about having this as a separate library or not, since Erick and Luan seem to prefer that.
And I had a more thorough look at gdx-ai today, and it is massive!

The pros and cons I see are the following (please fill in the list with things I miss):

As two packages gdx-ai (maybe named something fitting our "naming convention") and flame_gdx_ai:

  • + The code in gdx-ai can be re-used in any dart project.
  • + It can be released whenever deemed necessary.
  • - More maintenance overhead since two more libraries need to be released.
  • - Possibly docs, but the plan is to have all docs in the flame docs I think.

Inside of Flame:

  • + Easier to use for our users.
  • - Possibly a lot more code in the main package.

If we have a pure Dart gdx-ai port we can have you as the maintainer of that lib under the flame-engine org, if that is something you'd be interested in @st-pasha?

@st-pasha
Copy link
Contributor Author

Here's how I view the breakdown of various factors:

Inside of Flame

  • + Easier to use for our users
    This is a somewhat weak benefit: sure, it's much easier to depend on a single package than on three different ones, while also keeping up-to-date with upgrading their versions, etc. But it's not too much work.
  • +(+) Discoverability
    If the code is inside Flame, then it will be documented and the users will know that it's available for use. If the code is outside then it's much harder to discover. This warrants 1 + if we advertise the library on the main page of Flame docs, and 2 +s if we don't.
  • ++ Trust
    Any external package that the user might want to use, needs to be vetted. Is it trustworthy? Is it actively maintained? Is it supported by a single developer or a team (bus factor)? Is it well documented? etc. Being inside Flame, all of these qualities are automatic.
  • o A lot more code in the main package
    In some languages, having lots of code in the library is problematic, because the library is compiled into a single executable, and that executable is then used by the downstream users. However, Dart is not such a language. The tree-shaking algorithm ensures that the users who don't use these features will not be burdened by them.
  • o The code will need to be maintained
    I understand how this could be perceived as a -, but in reality the code will need to be maintained anyway -- whether it'll be in Flame or in some other repo. And maintaining it inside Flame is easier, because there will be less temptation to "sweep it under the rug".
  • +++ Ease of development
    This is a huge benefit from the developer standpoint. Flame has existing system for publishing examples/documentation. The Flame engine is needed for running tests. Compared to the alternatives, it's much easier to make changes in one package that are needed in another (this is currently a major pain point with our existing packages mock_canvas and ordered_set).
  • ++ Flame is perceived as a more powerful library
    The users will appreciate that Flame can fulfill all or most of their needs for game development, without them having to "shop for extra parts". This is like a difference between renting a fully-furnished apartment, and a bare-walls one. In practice you can think of it like this: in the popularity context between different libraries, people try to use at easily available metrics: stars on github, mentions on StackOverflow, downloads on pub.dev etc -- spreading the functionality across many small packages makes each of them appear small and insignificant, whereas taken together they are much more powerful.

Externally

  • + The code can be reused in other Dart projects
    This is a weak benefit: the code is useful for games only. And it can be reused when it is inside Flame too, though there could be a perception that it is somehow "too heavy". It is not, because of the tree-shaking.
  • - Separate release process
    I'm putting this as a - because Flame is already committed to regular releases, so compared to the first option there is little to be gained by having a separate release process. However, release process has its costs, and these costs will now have to be paid in full.
  • -- Documentation
    Currently we have a working system for publishing Flame documentation for each version (including examples). There is no comparable system for all other packages (even those in the mono-repo). Until such system is developed, this would be a major drawback.
  • There could also be different variants for the dependence structure:
    • Flame depends on this ai library
      • - Adds an external dependency to Flame
        External dependencies should generally be avoided, since they increase perceived size and risk profile of the main library. Say, a user may trust the Flame team, but how sure can they be in our 3rd-party vetting process?
    • Add a bridge library between flame and the ai library
      • -- The need to support another library
        Supporting a new library has substantial costs. In this scenario we'll be supporting one additional library, so that's extra costs to the maintainers.
      • --- Code duplication
        Since main Flame has the need for some of the ai behaviors (specifically for Camera movement), we'll end up creating the same functionality twice. And because this functionality will leave in separate classes, the end user will have to pay this cost too, as the functions will end up included twice in the final executable.

Summary

In my view, the benefits of including this functionality into Flame far outweigh the alternative of releasing the package separately. Of course, many of these arguments are somewhat speculative -- but many are drawn on the existing examples of packages that we have as externals, and which present significant challenges to the development.

If anything, we should start thinking of how we can bring our many disparate packages under a single umbrella (at least into the mono-repo).

@spydon
Copy link
Member

spydon commented Dec 30, 2021

Here's how I view the breakdown of various factors:

Inside of Flame

* **`++` Trust**

The package would live under the same organization, and that would be clear on pub, so I don't think this is an issue.

* **`o` The code will need to be maintained**
  I understand how this could be perceived as a `-`, but in reality the code will need to be maintained anyway -- whether it'll be in Flame or in some other repo. And maintaining it inside Flame is easier, because there will be less temptation to "sweep it under the rug".

If the full gdx-ai library should be ported it is probably close to 10k lines of code, it feels like that deserves its own pipeline, but I agree, the code does still need to be maintained.

* **`+++` Ease of development**
  This is a huge benefit from the developer standpoint. Flame has existing system for publishing examples/documentation. The Flame engine is needed for running tests. Compared to the alternatives, it's much easier to make changes in one package that are needed in another (this is currently a major pain point with our existing packages `mock_canvas` and `ordered_set`).

The plan is to merge all examples and docs, so I don't think this deserves three +. I have not had any pain points regarding this when developing things in ordered_set, quite the opposite, since it is contained in its own repo and doesn't need Flame the pipeline it is ten times faster. Can you elaborate on why you think it is a pain point?

* **`++` Flame is perceived as a more powerful library**
  The users will appreciate that Flame can fulfill all or most of their needs for game development, without them having to "shop for extra parts". This is like a difference between renting a fully-furnished apartment, and a bare-walls one. In practice you can think of it like this: in the popularity context between different libraries, people try to use at easily available metrics: stars on github, mentions on StackOverflow, downloads on pub.dev etc -- spreading the functionality across many small packages makes each of them appear small and insignificant, whereas taken together they are much more powerful.

If we document it well I don't think that Flame will be perceived as less powerful, and I don't think we'd get less stars on github on the monorepo because we have this separation. I mean, we don't even have audio support in the core library.

Externally

* **`+` The code can be reused in other Dart projects**
  This is a weak benefit: the code is useful for games only. And it can be reused when it is inside Flame too, though there could be a perception that it is somehow "too heavy". It is not, because of the tree-shaking.

A lot of people have used Forge2D outside of Flame and I think this would fall under many of the same use cases as Forge2D.
We of course know about tree-shaking, what we don't want is a big monolith. Even gdx-ai doesn't require libgdx to be used.

* **`-` Separate release process**
  I'm putting this as a `-` because Flame is already committed to regular releases, so compared to the first option there is little to be gained by having a separate release process. However, release process has its costs, and these costs will now have to be paid in full.

It is not hard to do a release to pub, you simply write pub publish (if you don't use melos) and then it is done. With a separate release schedule it can be released faster than we'd do flame releases, so I would definitely put this as a +. It will be much harder to cherry pick patches and do fast releases when it is within the core.

* **`--` Documentation**
  Currently we have a working system for publishing Flame documentation for each version (including examples). There is no comparable system for all other packages (even those in the mono-repo). Until such system is developed, this would be a major drawback.

See reply on "Ease of development".

* There could also be different variants for the dependence structure:
  
  * **Flame depends on this ai library**

This is not an option I think.

  * **Add a bridge library between flame and the ai library**

This is the option that we are discussing? (My suggestion to have it as only one package earlier was because I thought that we could move it in once it reached stability)

    * **`--` The need to support another library**
      Supporting a new library has substantial costs. In this scenario we'll be supporting one additional library, so that's extra costs to the maintainers.

What are the substantial costs that you see here?

    * **`---` Code duplication**
      Since main Flame has the need for some of the ai behaviors (specifically for Camera movement), we'll end up creating the same functionality twice. And because this functionality will leave in separate classes, the end user will have to pay this cost too, as the functions will end up included twice in the final executable.

Since the bridge library would depend on Flame we could expose an interface for different camera movements. I agree that we don't want code duplication.

Summary

... which present significant challenges to the development.

What are these significant challenges?

If anything, we should start thinking of how we can bring our many disparate packages under a single umbrella (at least into the mono-repo).

The umbrella is blue-fire and flame-engine, packages that can be used without Flame should not live within the monorepo.

Before I knew that the gdx-ai package was so big I thought it was a pretty good plan to put it within the core, but know I'm leaning towards @erickzanardo's and @luanpotter's standpoint to have it as a separate package.

@st-pasha
Copy link
Contributor Author

The package would live under the same organization, and that would be clear on pub, so I don't think this is an issue.

Maybe it's just me. But it took me more than 2 months to realize that ordered_set belongs to BlueFire team; and I just learned yesterday that audioplayers does too.

The plan is to merge all examples and docs,

Obviously this is a great plan, deserves a 👍 or even :+2:. But for now it is still more of an idea than an actual plan. I mean, there is no tracking issue, or a project, or an approximate estimate for how long it would take, or how much effort will be required, or commitment from specific people to do it. Surely, it can all magically materialize before the New Year, but it could also take several months or even quarters.

I have not had any pain points regarding this when developing things in ordered_set, quite the opposite, since it is contained in its own repo and doesn't need Flame the pipeline it is ten times faster. Can you elaborate on why you think it is a pain point?

Here's an actual example from canvas_test that I've encountered. Suppose I'm working on a new Flame feature, adding a test to draw something on a canvas -- and it turns out that there is no MockCanvas.drawCircle method. Ideally, if MockCanvas belonged to the flame_test repository, I would have went in and added the missing method in 5 minutes.

Instead, I'd need to (1) fork the canvas_test repository, (2) set up Android Studio for the new project, (3) implement the drawCircle functionality, including tests, (4) make a PR, (5) wait for the PR to be approved and merged, (6) make another PR for the new release of canvas_test (actually I'm not entirely sure how to do this part), (7) wait for the PR to be approved and merged (optionally: spend some time arguing whether a single function is release-worthy or maybe it should be released later), (8) make a third PR in the main flame repository bumping the version of the canvas_test dependency, (9) wait for that PR to be approved and merged (hopefully quicker than the first two), (10) pull these changes into the feature branch that you were originally working on -- only now I'm finally unblocked to work on my feature.

Note that most of these concerns wouldn't apply if the ai library was inside the mono-repo and flame depended on it via relative import.

A lot of people have used Forge2D outside of Flame and I think this would fall under many of the same use cases as Forge2D. We of course know about tree-shaking, what we don't want is a big monolith. Even gdx-ai doesn't require libgdx to be used.

I wonder what's their use-case though? Developing game engines alternative to Flame? Do we even want to encourage that?

For gdx-ai I suspect their prime motivator is to make their engine jar files smaller, so that developers who don't need AI capabilities wouldn't have to carry it along. After all, Java doesn't have tree-shaking like Dart.

It is not hard to do a release to pub, you simply write pub publish and then it is done.

There is always some hidden cost of publishing, and in general maintaining the CI infrastructure. Creating release branches and tags, making GitHub release, resolving deprecation events, updating readme, creating changelog, re-building documentation, making public announcements, etc.

@st-pasha
Copy link
Contributor Author

The umbrella is blue-fire and flame-engine, packages that can be used without Flame should not live within the monorepo.

We can always design the package in such a way that it cannot be used without Flame. Ultimately, this AI engine is for use in games, it has no external purpose like, say, audioplayers.


Ultimately, though, is the question of scope: is this functionality within the scope of Flame engine?

Suppose, for example, that there was no gdxAI project. I came in and say: look, we have position-components which can be placed anywhere, and we also have move effects that can move them in a predefined way, and even a "speed" parameter that can change the effect based on the initial position of the target. But wouldn't it be nice if we also added more complicated types of behavior, where targets update their trajectory continuously, where they can react to other targets or to the environment?

How do we decide what's in Flame's scope and what's outside?

If the concern is that the code originates from gdxAI -- then we can develop it independently. It would still be easier than maintaining a separate project.

If the concern is code size (and btw, Java is about twice more verbose than Dart), then we can limit the amount of things we are going to implement.

If the concern is that the code can in theory be separated, then I don't understand what's the scope of the Flame library. Because it has plenty of things that could have been in separate packages (like particles, effects, collisions, etc).

@erickzanardo
Copy link
Member

For me the reason for this being a package on its own is pretty simple, this is a port of another lib, better keep its code base apart from Flame.

@st-pasha
Copy link
Contributor Author

For me the reason for this being a package on its own is pretty simple, this is a port of another lib, better keep its code base apart from Flame.

So, would you be ok with including this (or equivalent) functionality into Flame, if it was developed independently from gdx-ai or any other existing library?

@spydon
Copy link
Member

spydon commented Jan 2, 2022

Maybe it's just me. But it took me more than 2 months to realize that ordered_set belongs to BlueFire team; and I just learned yesterday that audioplayers does too.

They were moved over quite recently from Luan's personal account, if anyone is actually concerned about the origin of a package it is very easy to see that they come from us now though.

Obviously this is a great plan, deserves a +1 or even :+2:. But for now it is still more of an idea than an actual plan. I mean, there is no tracking issue, or a project, or an approximate estimate for how long it would take, or how much effort will be required, or commitment from specific people to do it. Surely, it can all magically materialize before the New Year, but it could also take several months or even quarters.

It can be started for new projects already, so all gdx-ai docs would be written just like if they would have been included in the core.

Instead, I'd need to (1) fork the canvas_test repository, (2) set up Android Studio for the new project, (3) implement the drawCircle functionality, including tests, (4) make a PR, (5) wait for the PR to be approved and merged, (6) make another PR for the new release of canvas_test (actually I'm not entirely sure how to do this part), (7) wait for the PR to be approved and merged (optionally: spend some time arguing whether a single function is release-worthy or maybe it should be released later), (8) make a third PR in the main flame repository bumping the version of the canvas_test dependency, (9) wait for that PR to be approved and merged (hopefully quicker than the first two), (10) pull these changes into the feature branch that you were originally working on -- only now I'm finally unblocked to work on my feature.

I can see your point, 6 and 7 doesn't work like that though, we publish a new version and then we do the PR afterwards. And 8-10 could be done in the same PR as where you need the changes and you could have your PR depend on your branch before it is merged. For these kind of libraries which can develop on their own we definitely don't want them inside of the monorepo, these can live completely without flame and if someone wants to continue work on them they can simply fork only the small repo instead of the full monorepo.

I wonder what's their use-case though? Developing game engines alternative to Flame? Do we even want to encourage that?

Of course, anything that gives our users freedom to come up with whatever crazy ideas they have. One person for example created a site with directed graphs and each entity was controlled with a Forge2D body and springs, completely outside of Flutter. We don't want to lock in people to Flame and Flutter when we don't need to, if it isn't making our lives too hard, so that is what we are really discussing. A pure dart library with gdx-ai could be used with so much more than just Flame, compare with the a-star library for example.

There is always some hidden cost of publishing, and in general maintaining the CI infrastructure. Creating release branches and tags, making GitHub release, resolving deprecation events, updating readme, creating changelog, re-building documentation, making public announcements, etc.

Almost all of that should be automated though, and just as much work outside the monorepo as within it. We have generalized most of the pipeline actions so that they can be used in different repositories.

How do we decide what's in Flame's scope and what's outside?

This is a very good question! I'll make sure that we discuss that on our next meeting!
(Everyone has been a bit off now during the holiday, but we should be having a meeting soon)
It might sound like I'm completely against having this in the mainrepo, but I'm not, I just want to weigh all the pros and cons.

@st-pasha
Copy link
Contributor Author

st-pasha commented Jan 2, 2022

Ok, I surrender. I'll create a new package.

@st-pasha st-pasha closed this Jan 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants