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

Miscellaneous fixes for headless legacy #362

Merged
merged 4 commits into from
Oct 30, 2024

Conversation

lesquoyb
Copy link
Contributor

@lesquoyb lesquoyb commented Oct 24, 2024

Starting by fixing #352: the problem happened when reaching the exportVariable function and trying to serialize the displays.
For now I'm just handling the case where the dataType variable is not defined.
But the issue appeared on Model 12.gaml from the library of models, in which the displays are explicitly defined as 2d, so shouldn't dataType be DataType.DISPLAY2D when entering this function? And if so, what is the expected dataType in case of 3d display ?
@AlexisDrogoul any pointer ?

For now I'm just handling the case where the dataType variable is not defined.
But if we run headless legacy on Model 12.gaml from model library, we define the displays as 2d explicitly, so shouldn't dataType be 2d display ? And if so, what is the expected dataType in case of 3d display ?
Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

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

Code Health Quality Gates: OK

Change in average Code Health of affected files: +0.00 (10.00 -> 10.00)

View detailed results in CodeScene

@AlexisDrogoul
Copy link
Member

I have no idea. I have never really looked into this part of the code ...

@lesquoyb lesquoyb changed the title Addresses #352 Miscellaneous fixes for headless legacy Oct 30, 2024
Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

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

Code Health Quality Gates: FAILED

Change in average Code Health of affected files: -0.01 (7.70 -> 7.69)

  • Declining Code Health: 1 findings(s) 🚩

View detailed results in CodeScene

@lesquoyb
Copy link
Contributor Author

I dug a bit and this part of the code is basically only called in legacy mode (and maybe some check mode) so as long as the initial bug is fixed and there's no new weird behaviour in legacy I think it's safe to merge.

While digging I found a few other issues that I patched in the following commits, the most important was restoring the -gaml mode that basically already did what is asked in #361

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

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

Code Health Quality Gates: FAILED

Change in average Code Health of affected files: -0.02 (8.06 -> 8.04)

  • Declining Code Health: 1 findings(s) 🚩

View detailed results in CodeScene

@@ -121,6 +121,9 @@ private static Injector configureInjector() {
/** The Constant THREAD_PARAMETER. */
final public static String THREAD_PARAMETER = "-hpc";

/** The Constant THREAD_PARAMETER. */

Choose a reason for hiding this comment

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

ℹ Getting worse: Overall Code Complexity
The mean cyclomatic complexity increases from 4.79 to 5.00, threshold = 4

@lesquoyb lesquoyb merged commit 469f87e into 2024-06 Oct 30, 2024
5 of 6 checks passed
@lesquoyb lesquoyb deleted the fixes-headless-variable-export branch October 30, 2024 10:05
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.

2 participants