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

Fix accidental globals and references to undeclared variables #819

Merged
merged 1 commit into from
Aug 21, 2022

Conversation

KiwiHawk
Copy link
Collaborator

@KiwiHawk KiwiHawk commented Jul 31, 2022

Found by temporarily adding the following code to angelsrefining/data.lua

setmetatable(_G, {
  __newindex = function (_, n)
    error("attempt to write to undeclared variable "..n)
  end,
  __index = function (_, n)
    error("attempt to read undeclared variable "..n)
  end,
})

@LovelySanta
Copy link
Collaborator

Should 'similar' code be added to angels unit tests to log these kinds of errors?

@KiwiHawk
Copy link
Collaborator Author

KiwiHawk commented Aug 3, 2022

Possibly. Personally, I would prefer if this check is a separate new mod. The missing recipe tints check could be part of that new mod too.

Reason being they are very different type of checks: data stage rather than control.

Mod load order is important. The metatable thing needs to be setup, ideally before any other mods run.

If we wanted some kind of summary printout to the log file, then we'll need two new mods. One that runs first (before any other mods) and the other runs last (after all other mods).

@LovelySanta
Copy link
Collaborator

I agree, the angelsdev-unit-test should run last (so all on_init stuff from other mods and the whole event dispatching order goes to other mods first prior to the unit tests)...

This can easily be done by adding hidden optional dependencies to the unit test mod to depend on all other (relevant) mods...
Then we need another small mod (I was thinking of angelsdev-unit-test-setup) that runs before other mods:

  • For angels this is easy by adding a hidden optional dependency on the new mod.
  • Add it as prerequisite to a minimal amount of bobmods?

The angelsdev-unit-test-setup should log on its own when the angelsdev-unit-test is present.

On a side note: checking data stage should also be done in angelsdev-unit-test as all mods must have had the chance of using data-final-fixes before we check it....

@KiwiHawk
Copy link
Collaborator Author

KiwiHawk commented Aug 7, 2022

You can add reverse dependencies on other mods (for lack of a better term). This forces the other mod to load after. Prefix for this is ~. I remember reading about it but I've not been able to find any documentation on it.

This would be the ideal, as long as it can be made optional. Then all the dependencies could be in the new mod.

Otherwise yeah, it could be added as a hidden optional prerequisite to boblibrary.
Alternatively we could just rely on the alphanumeric sort order. Mod "AAAAAAAAAAAAA Test" should be loaded first, right?

This discussion should probably be moved to a separate, new issue though.

@KiwiHawk
Copy link
Collaborator Author

@LovelySanta I've created a new issue, logging your suggestion for new unit tests. So hopefully this PR can be merged now? :-)

@LovelySanta LovelySanta merged commit 8142e0c into Arch666Angel:dev Aug 21, 2022
@KiwiHawk KiwiHawk deleted the accidental-globals branch August 21, 2022 21:10
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