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

Repr improvement #6597

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from
Open

Repr improvement #6597

wants to merge 20 commits into from

Conversation

lL1l1
Copy link
Contributor

@lL1l1 lL1l1 commented Dec 22, 2024

Description of the proposed changes

More FAF-specific improvements to the inspect library:

  • Print the base class of a table, if any.
  • Don't print __index fields if they're the same as the table. This removes a lot of common recursive clutter (it is still possible to recurse, for example the Brain.CDR.Brain or WorldView.view.WorldView in the example below).
  • Print source for lua functions in a rather short local format.

Testing done on the proposed changes

Use this command on a Percival:

SimLua LOG(repr(SelectedUnit(), {meta=true}))

Example Output (Percival).txt

Use this command:

UI_Lua LOG(repr(import('/lua/ui/game/worldview.lua').GetWorldViews()['WorldCamera'], {meta = true}))

Example Output (main WorldView).txt

Additional context

The last version of the original repr printed class-specific data. I was lazy on implementing that, but it could be done by using a helper function instead of directly the names table. Still, in general these changes lay the foundations for class-specific data printing.

Checklist

  • Changes are annotated, including comments where useful
  • Changes are documented in the changelog for the next game version

@lL1l1 lL1l1 force-pushed the repr-improvement branch 4 times, most recently from 72c4aed to 47aac38 Compare December 22, 2024 09:25
@lL1l1 lL1l1 marked this pull request as ready for review December 22, 2024 09:28
@Garanas
Copy link
Member

Garanas commented Dec 25, 2024

@maudlin27 / @relent0r you both use this function too. It may be interesting for you to look at these changes and share your thoughts.

lua/system/repr.lua Outdated Show resolved Hide resolved
lua/system/repr.lua Outdated Show resolved Hide resolved
lua/system/repr.lua Show resolved Hide resolved
@lL1l1 lL1l1 requested a review from 4z0t December 27, 2024 12:00
lua/system/repr.lua Outdated Show resolved Hide resolved
@Garanas
Copy link
Member

Garanas commented Jan 25, 2025

These changes are fine in general. It appears however that calling reprsl to a unit generates a very large print. What if we by default skip (recursively) printing tables with a metatable, with the exception of course of the table that we're printing initially?

The reason is that at the moment the Brain reference is recursively printed too. And that can hold a lot of information, especially for AIs.

@relent0r
Copy link
Contributor

For the uninitiated what does this do for us exactly compared to the repr function now?

@lL1l1
Copy link
Contributor Author

lL1l1 commented Jan 26, 2025

Maybe it is better to skip only brains and blueprints (if they're not the root), and create a flag for that specifically? There could be a wide variety of tables with metatables that we'd want to avoid.

Alternatively the proper solution would be to be like deepcopy and have a "backrefs" table that tells us whether we've seen a table or not before.

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