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/caspar ai #6820

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

Feat/caspar ai #6820

wants to merge 16 commits into from

Conversation

Scoppio
Copy link
Collaborator

@Scoppio Scoppio commented Apr 3, 2025

Adds basic stuff needed for Caspar, and updates the games action log.

@Scoppio Scoppio self-assigned this Apr 3, 2025
@@ -33,7 +33,7 @@ private MathUtility() {
* maximum. Otherwise, this will return the rounded integer between the
* two points
*/
public static int lerp(final int min, final int max, final double f) {
public static int lerp(int min, int max, double f) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

final is useless in arguments, so I just removed it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's actually not useless. It prevents the programmer from re-assigning a value to them.

Copy link

codecov bot commented Apr 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 30.08%. Comparing base (baf3842) to head (d7f4627).
Report is 7 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6820      +/-   ##
============================================
+ Coverage     30.06%   30.08%   +0.01%     
- Complexity    15597    15633      +36     
============================================
  Files          2872     2877       +5     
  Lines        282720   282836     +116     
  Branches      49259    49258       -1     
============================================
+ Hits          85002    85084      +82     
- Misses       192154   192187      +33     
- Partials       5564     5565       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…time, need to change it so it just fails if value is higher than a certain percent
Copy link
Collaborator

@rjhancock rjhancock left a comment

Choose a reason for hiding this comment

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

How often will the binary files change?

Other than a few comments and a few classes could use (not required) some top level JavaDocs, it looks good for the initial PR.

@@ -33,7 +33,7 @@ private MathUtility() {
* maximum. Otherwise, this will return the rounded integer between the
* two points
*/
public static int lerp(final int min, final int max, final double f) {
public static int lerp(int min, int max, double f) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's actually not useless. It prevents the programmer from re-assigning a value to them.

@Scoppio
Copy link
Collaborator Author

Scoppio commented Apr 3, 2025

How often will the binary files change?

Other than a few comments and a few classes could use (not required) some top level JavaDocs, it looks good for the initial PR.

It depends... Ideally I want to create at least 5 AI models (Beginer, easy, normal, hard, hardcore), and maybe two or three "flavors" for each of those too would be interesting (if viable).

I may try to make them read directly from zip files, or at least tar all the model files into one thing instead? Need to check Tensorflow's API. But the models sohuld not change alot once done, unless someone is doing anything with them, like adding new stuff or tuning, in any way, its usually just a file replace.

@rjhancock
Copy link
Collaborator

Alright. The speed of change was my concern as the way Git handles binary is it just re-adds the entire file. Not changing often, not really an issue.

@Scoppio Scoppio added the For New Dev Cycle This PR should be merged at the beginning of a dev cycle label Apr 5, 2025
@Scoppio Scoppio requested a review from Copilot April 6, 2025 22:24
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 25 out of 29 changed files in this pull request and generated 1 comment.

Files not reviewed (4)
  • megamek/build.gradle: Language not supported
  • megamek/data/ai/brains/default/fingerprint.pb: Language not supported
  • megamek/data/ai/brains/default/keras_metadata.pb: Language not supported
  • megamek/data/ai/brains/default/min_max_feature_normalization.csv: Language not supported
Comments suppressed due to low confidence (1)

megamek/src/megamek/ai/neuralnetwork/Brain.java:128

  • [nitpick] The error message for a failed prediction is unclear and may be confusing. Consider revising the message to something more informative like 'Prediction failed due to an unexpected error' for improved clarity.
logger.error("Prediction failed, there is no recourse from this", e);

* @return The value clamped
*/
public static float clamp01(float value) {
return clamp(value, 0f, 1f);
Copy link
Preview

Copilot AI Apr 6, 2025

Choose a reason for hiding this comment

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

The clamp01 methods are calling a clamp method with float/double arguments, but the existing clamp method appears to operate on long values. Consider adding overloads of clamp for float and double types or casting the arguments appropriately to ensure type consistency.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For New Dev Cycle This PR should be merged at the beginning of a dev cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants