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

general overhaul for 5.0.0 #11

Merged
merged 4 commits into from
Oct 14, 2024
Merged

general overhaul for 5.0.0 #11

merged 4 commits into from
Oct 14, 2024

Conversation

SwissalpS
Copy link
Contributor

@SwissalpS SwissalpS commented Sep 18, 2024

  • overhauled English language
  • overhauled French language
  • adds Spanish locale
  • adds German locale
  • adds mtt integration testing
  • adds github workflow for mtt
  • refactores handle_command
  • standardizes usage of table.getn() (nothing crucial in this mod)
  • standardizes variables to have same format and same name for same purpouse
  • DB-fix also adds missing owners to members
  • fixes some situations where admins couldn't do things
  • fixes display of dynamic admin priv and registration
  • alows uppercase for subcommands
  • adds valid_password() for cleaner checks
  • ensures that existing factions don't get overwritten (via 'API')
  • adds disband-hook registration for other mods to get notified of disbanded factions

- overhauled English language
- overhauled French language
- adds Spanish locale
- adds German locale
- adds mtt integration testing
- adds github workflow for mtt
- refactores handle_command
- standardizes usage of table.getn() (nothing crucial in this mod)
- standardizes variables to have same format and same name for same
purpouse
- DB-fix also adds missing owners to members
- fixes some situations where admins couldn't do things
- fixes display of dynamic admin priv and registration
- alows uppercase for subcommands
- adds valid_password() for cleaner checks
- ensures that existing factions don't get overwritten
@SwissalpS SwissalpS added the enhancement New feature or request label Sep 18, 2024
@SwissalpS
Copy link
Contributor Author

Interesting that mtt workflow doesn't have table.pack()

This was referenced Sep 18, 2024
@SwissalpS SwissalpS marked this pull request as ready for review September 18, 2024 01:56
@BuckarooBanzay
Copy link
Member

Interesting that mtt workflow doesn't have table.pack()

err, the mtt workflow uses the upstream minetest image, should always be ghcr.io/minetest/minetest:master do you have table.pack in your local environment? 🤔

@BuckarooBanzay BuckarooBanzay mentioned this pull request Sep 23, 2024
@SwissalpS
Copy link
Contributor Author

I'm using Lua 5.4.6 table.pack seems to be part of that:
https://www.lua.org/manual/5.4/manual.html#pdf-table.pack

@TheEt1234
Copy link

TheEt1234 commented Sep 23, 2024

table.pack is not supported on luajit (luajit is based upon lua5.1 but heavily extended), and i think non-luajit minetest uses lua5.1 which doesn't support it either (but like 99% of minetest users have the luajit version anyway)

When executing //lua minetest.log(dump(table.pack)) just it logs nil

@wsor4035
Copy link
Contributor

I'm using Lua 5.4.6 table.pack seems to be part of that: https://www.lua.org/manual/5.4/manual.html#pdf-table.pack

which is irrelevant given minetest uses 5.1 lua (some 5.2 features if on luajit)

@SwissalpS
Copy link
Contributor Author

SwissalpS commented Sep 24, 2024

I'm using Lua 5.4.6 table.pack seems to be part of that: https://www.lua.org/manual/5.4/manual.html#pdf-table.pack

which is irrelevant given minetest uses 5.1 lua (some 5.2 features if on luajit)

Why would it be irrelevant? Obviously my minetest build is using a version that offers table.pack. So according to above comments not using neither LuaJIT nor 5.1.

Edit: it is irrelevant now in the sense that it is only used in mtt tests and I've provided a working alternative.

@wsor4035
Copy link
Contributor

It's irrelevant since minetest officially only uses/supports lua5.1(and luajit), if you have compiled/obtained a build somewhere else thats non standard, that's on you.

mtt.lua Outdated
else
pd = function(...) for _, v in ipairs({ ... }) do print(dump(v)) end end
end
local f, fcc, S = factions, factions.handle_command, factions.S
Copy link
Member

Choose a reason for hiding this comment

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

I'd generally recommend against localizing stuff for tests so that it would fail also if some mistake removes/trashes/replaces global or its entries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, removed local reference f of factions. I left the other two as they are only made public when running mtt and if factions got deleted, errors would already be thrown.

Copy link
Contributor

@wsor4035 wsor4035 left a comment

Choose a reason for hiding this comment

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

didnt test, but did a read through of the code. seems good enough.

some renames, etc i dont really care about, maybe someone else might, but 🤷

@wsor4035 wsor4035 merged commit c629b29 into master Oct 14, 2024
4 checks passed
@SwissalpS SwissalpS deleted the updated_5.0.0 branch October 14, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants