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: update rect, text, color, and hooks #1075

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

Conversation

Wintermourn
Copy link
Contributor

This PR contains changes that bring my code up-to-date with my current standards and skills, adding in some new features and hopefully (slightly) less memory usage.

rect, color, text, and hook have been separated into their own files, all loaded together through external.script.lua-tools.init.
rect, color, and text now have their own shared metatables instead of making a new one for each instance.

  • default values are automatically used.
  • text:create and text:new are now text.create and text.new.
  • color:new, color:fromHex are now color.new and color.fromHex.
  • rect:create is now rect.create.

rect:update(t) now checks the key color, which will update it's r, g, b, src, and dst values.
color:toHex is shorter, using string.format.

New functions for hooks:

  • hook.on(list, callback): adds a callback to a hook list without a name.
  • hook.removeOn(list, callback): removes a callback from the recurring list, named or not.
  • hook.once(list, callback): adds a callback to a hook list which only runs once.
  • hook.removeOnce(list, callback): removes a one-time callback. Probably only useful if one mod's trying to cut another off.

One time callbacks are run before recurring ones.

New functions for colors:

  • color.fromHSL(h, s, l, src, dst): Creates a color using the HSL format.
  • Color:toHSL(): Returns h, s, l, src, and dst values.

Another file has been made to store extra functions that extend existing parts of Lua, currently only including table.getKey(tbl, value).

@github-actions github-actions bot added the PR: feat This PR implements a new feature label Mar 20, 2023
@K4thos
Copy link
Member

K4thos commented Mar 20, 2023

It looks very good and can be merged once stable version 0.99.0 is published. However, please keep in mind that a major Lua refactoring is currently in progress. Therefore, the changes made to the current scripts and the old file structure will not be kept once that branch is merged. For this reason, I am not encouraging people to work with the current scripts and instead wait for the refactoring to be finished. But of course I will implement expanded functionality and improvements from this PR in the refactored code.

The general approach is similar to yours but scope of object-oriented changes affects most of the code and unified commenting allows for proper integration with VSC and IntelliJ IDE. Currently, the scripts have been separated into 41 classes and subclasses with inheritance, each in a different file, and more will be added. Here is an example of your old color functions following the style and commenting that the Lua refactored code strictly follows: https://gist.github.com/K4thos/6893e5b458cbfa1358f318988199a242

@K4thos K4thos added the target: minor This PR is targeted for the next minor release label Mar 20, 2023
@Wintermourn
Copy link
Contributor Author

I think I remember hearing something about the refactoring a while ago but I didn't know whether it was being worked on or if it was public. The code and comments look nice, and I look forward to seeing and working with the rest of it.
I am a bit confused by the code making a global variable and then returning that. Is that intentional?

@K4thos
Copy link
Member

K4thos commented Mar 20, 2023

That last return can be most likely skipped but not using local for this is intentional. Without global either VSC or IntelliJ extension that this type of commenting is aimed at wasn't able to provide all hints, suggest types, code completion or following functions between files, if I remember correctly, but I don't remember the details right now. Once the refactoring is done I hope that you will help me to improve it since you're really good with Lua.

@K4thos K4thos added target: major This PR is targeted for the next major release and removed target: minor This PR is targeted for the next minor release labels Jun 10, 2023
@K4thos K4thos changed the base branch from master to develop November 1, 2023 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: feat This PR implements a new feature target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants