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.

Sign up for GitHub

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

Jump to bottom

feat: add Goal #7

Merged
merged 11 commits into from
Mar 17, 2024
Merged

feat: add Goal #7

merged 11 commits into from
Mar 17, 2024

Conversation

SilasPeters
Copy link
Contributor

@SilasPeters SilasPeters commented Feb 27, 2024 •
edited
Loading

All features we considered are implemented. However, the Java implementation also mentioned float distance as a heuristic value, and Object proposal as the potential solution to the goal.

The idea of heuristic values has been included, as the BDI cycle needs this.

The concept of a "proposal" clashes with both Unity and our requirement to leave out World Models, and thus this PR excludes this.

@SilasPeters SilasPeters added enhancement New feature or request question Further information is requested labels Feb 27, 2024
@SilasPeters SilasPeters self-assigned this Feb 27, 2024
@SilasPeters SilasPeters changed the title Add first draft of Goal Add Goal Feb 27, 2024
@JensSteenmetz JensSteenmetz changed the title Add Goal feat: add Goal Feb 27, 2024
@SilasPeters SilasPeters marked this pull request as ready for review February 28, 2024 15:45
SSelinn
SSelinn reviewed Mar 4, 2024
View reviewed changes
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
joachimdekker
joachimdekker requested changes Mar 4, 2024
View reviewed changes
Copy link
Contributor

@joachimdekker joachimdekker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice foundation! I have left a couple of comments, ranging from simple nitpicks/docs to critical questions.

Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Tests/Aplib.Tests.csproj Outdated Show resolved Hide resolved
Aplib.Tests/Desire/GoalTests.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Tactic.cs Show resolved Hide resolved
SSelinn
SSelinn suggested changes Mar 4, 2024
View reviewed changes
Copy link
Collaborator

@SSelinn SSelinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments, but it looks fine to me otherwise

JensSteenmetz
JensSteenmetz requested changes Mar 4, 2024
View reviewed changes
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/IGoalPredicate.cs Outdated Show resolved Hide resolved
Aplib.Tests/Desire/GoalTests.cs Outdated Show resolved Hide resolved
Aplib.Tests/Desire/GoalTests.cs Outdated Show resolved Hide resolved
Aplib.Tests/Desire/GoalTests.cs Outdated Show resolved Hide resolved
@SilasPeters SilasPeters force-pushed the feat/0004/Goal branch 2 times, most recently from 7308bcc to 9e637d7 Compare March 6, 2024 13:50
@SilasPeters SilasPeters removed the question Further information is requested label Mar 10, 2024
joachimdekker
joachimdekker requested changes Mar 12, 2024
View reviewed changes
Copy link
Contributor

@joachimdekker joachimdekker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few nitpicks. Rest looks good!

Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Goal.cs Outdated Show resolved Hide resolved
Aplib.Core/Desire/Tactic.cs Show resolved Hide resolved
SSelinn
SSelinn previously approved these changes Mar 13, 2024
View reviewed changes
Aplib.Core/Desire/Tactic.cs Show resolved Hide resolved
SSelinn
SSelinn previously approved these changes Mar 13, 2024
View reviewed changes
Copy link
Collaborator

@SSelinn SSelinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TUTA (Thumbs Up To All)

JensSteenmetz
JensSteenmetz previously approved these changes Mar 13, 2024
View reviewed changes
Copy link
Member

@JensSteenmetz JensSteenmetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nltho (nothing left to hate on)

@SilasPeters SilasPeters dismissed stale reviews from JensSteenmetz and SSelinn via 0feea39 March 13, 2024 15:15
JensSteenmetz
JensSteenmetz previously approved these changes Mar 13, 2024
View reviewed changes
Copy link
Member

@JensSteenmetz JensSteenmetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iosgc (I only see good code)

SSelinn
SSelinn previously approved these changes Mar 13, 2024
View reviewed changes
Copy link
Collaborator

@SSelinn SSelinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PLMFL (Presentation Looking Mighty Fine Lad)

JensSteenmetz
JensSteenmetz previously approved these changes Mar 13, 2024
View reviewed changes
Copy link
Member

@JensSteenmetz JensSteenmetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kutgw (keep up the good work)

@SilasPeters SilasPeters dismissed stale reviews from JensSteenmetz and SSelinn via 2fbce93 March 13, 2024 15:41
JensSteenmetz
JensSteenmetz approved these changes Mar 13, 2024
View reviewed changes
Copy link
Member

@JensSteenmetz JensSteenmetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iaotuh (I approve of this unparallelled humor)

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

SSelinn
SSelinn approved these changes Mar 13, 2024
View reviewed changes
Copy link
Collaborator

@SSelinn SSelinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OLAATP (Operation Looking All According To Plan)

@QuakeEye QuakeEye mentioned this pull request Mar 17, 2024
Merged
joachimdekker
joachimdekker approved these changes Mar 17, 2024
View reviewed changes
Copy link
Contributor

@joachimdekker joachimdekker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FTPRCBM! (Finally, This Pull Request Can Be Merged)

@SilasPeters SilasPeters merged commit f401280 into main Mar 17, 2024
2 checks passed
@SilasPeters SilasPeters deleted the feat/0004/Goal branch March 17, 2024 18:33
@github-actions GitHub Actions
Copy link

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SSelinn SSelinn SSelinn approved these changes

@joachimdekker joachimdekker joachimdekker approved these changes

@JensSteenmetz JensSteenmetz JensSteenmetz approved these changes

Assignees

@SilasPeters SilasPeters

Labels
enhancement New feature or request released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SilasPeters @JensSteenmetz @joachimdekker @SSelinn