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

C++ version #5

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

C++ version #5

wants to merge 23 commits into from

Conversation

amoldeshpande
Copy link

C++ Version PR

Description

Implements a C++ version of Fluid-HTN.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • [x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [x ] This change requires a documentation update

How Has This Been Tested?

Includes the Unit Tests (barring a couple which test for Nulls where the C++ version is passing references)
Also tested cursorily with the C++ port of the Text Adventure sample.

(https://github.com/accesshoops/fluid-text-adventure/tree/master/Fluid%20Text%20Adventure, separate PR to follow there)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • [x ] I have added tests that prove my fix is effective or that my feature works
  • [x ] New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

* Works like C# for the most part, except for DomainBuilder not using fluent style.
* Most Unit tests were ported, barring some which tested for null pointers in various places where the C++ version does not have them.
This was referenced Dec 8, 2020
amoldeshpande and others added 19 commits December 8, 2020 12:34
* Converted to using templates. There isn't a clean way to make a UE4 Plugin otherwise.  A typedef of WORLDSTATEPROPERTY_TYPE etc. is a bad idea because that makes the Plugin dependent on the main game code.

* make some BaseContext functions non-overrideable. These relate to the core state of a Context.

* - move headers into directories so that include paths must be specified from the root of the project directory. This keeps naming conflicts with other headers to a minimum by forcing a sort of "namespace"
- added some virtual destructors which UE4 complains about.

* - move pch back to root. It should not be included by consuming projects
- Moved STLTypes.h out of pch so that consumers are not forced to include pch.h

* remove all CPPs. IContext is a proper interface so there are no circular includes any more.
* replace vector as well since #defines (expectedly) cause havoc in UE4's unity builds.

* change preprocessor define to be less generic.

* task.h needs to include stlttypes.h

* fix compiler warning about void return value

* dont return references, could potentially be locals
* allow replacing std::rand
@fire
Copy link

fire commented Jul 17, 2021

I was thinking of integrating this in Godot Engine.

Doctest is better than GoogleTest and CPPUnit.

Not sure how updates from c# will work.

Any comments, questions or jokes?

@amoldeshpande
Copy link
Author

Any updates from C# have to be done manually, which is not a very sustainable approach, but adequate for my personal needs. Doesn't godot support C# ?

As far as test frameworks go, I dislike having to install additional software (especially with C++ where you don't have a nice clean story like NuGet) so I went with MSTest. Feel free to fork and do whatever you like though :)

@fire
Copy link

fire commented Jul 17, 2021

I personally don't use c#.

Godot supports C# but it's not the default and there are proposals to move C# support to a shared library rather than a secondary official build.

Are you aware that Doctest is a single c++ header and only the header is required in a program?

https://github.com/onqtam/doctest/blob/master/doctest/doctest.h

My goal of this conversation is to collaborate as much as possible.

@amoldeshpande
Copy link
Author

In the end, I think having a proper cross-platform test framework is a good idea. There is no valid reason to keep it tied to MSTest for ever, just my own preferences.
From that perspective, doctest is certainly more attractive than gtest because it would be header-only, just like the framework itself.

However, the project I did this port for is on the back burner and may end up being canned. If your intention is to have a well-supported version that is properly updated it may be best to fork my repo and maintain it yourself.

Not trying to be a jerk, I just cannot provide the commitment needed in order to be a good steward.

You can even make your own PR as a more sustainable C++ port and I will have no objection to closing this one.

2. Fixed: Improper adding to stackDepth, which causes unit test failed
@kthecoder
Copy link

In the end, I think having a proper cross-platform test framework is a good idea. There is no valid reason to keep it tied to MSTest for ever, just my own preferences. From that perspective, doctest is certainly more attractive than gtest because it would be header-only, just like the framework itself.

However, the project I did this port for is on the back burner and may end up being canned. If your intention is to have a well-supported version that is properly updated it may be best to fork my repo and maintain it yourself.

Not trying to be a jerk, I just cannot provide the commitment needed in order to be a good steward.

You can even make your own PR as a more sustainable C++ port and I will have no objection to closing this one.

@amoldeshpande Is it okay if I maintain a version of your c++ port for Godot and use it in my games? Do you want me to include any extra license for the code you wrote?

C++ Code lives at : https://github.com/kthecoder/AIBlue/tree/main/extension/src/FluidHTN
Tests live at : https://github.com/kthecoder/AIBlue/tree/main/test/cpp/fluidhtn_tests

I have also ported the tests to Google Test, because that was the test framework I could actually get to work on my computer lol.

@amoldeshpande
Copy link
Author

@kthecoder Do feel free to use the code any way you wish, without any additional licensing on my code. Good luck with your games!

@fire
Copy link

fire commented Feb 6, 2024

Thanks for the update - for me I ported the HTN framework from Dana https://github.com/dananau/GTPyhop

https://github.com/V-Sekai/godot-task-goal-planner

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.

None yet

4 participants