love
love copied to clipboard
Fix callback naming
Original report by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).
We should really find a better way of naming callbacks, just look at the joystick ones, the function names are getting long, and are hard to read. Related, why are they not camelCase?
Ideas are welcome!
Original comment by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).
#508 was marked as a duplicate of this issue.
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
Non-camelCase names just... feel nicer to me... I like them even though they're inconsistent... and I freakin' love consistency. One logical-esque reason for them being lowercase is that they "look" different from non-callback functions. But maybe I'm just used to them. :D
I really like how most LÖVE functions aren't abbreviated, their meaning is obvious and you can read them nicely. I think love.joystickpressed
would be a much nicer name than love.joyp
or something. Also I think the format of callbacks are important for recognising them, so I also don't think something like love.joystick.pressed
would be so great.
There could be a single love.joystick
callback which is given an extra argument for whether a button was pressed or released, but I'd suggest that would end up with recreations of the two callbacks using an if-else.
:::lua
function love.joystick(joystick, button, pressed)
if pressed then
...
else
...
end
end
This is basically what love.focus
does however. So, should love.focus
be split into love.focusreceived
and love.focuslost
? (Wow, those names really are a case for camelCase. :P) Edit: Maybe some better names: love.focuson
and love.focusoff
. I think it depends on whether it would be inevitably be completely divided with an if-else, like input callbacks would be.
Just to recap from #503, I currently think that love.errhand(msg)
and love.releaseerrhand(msg)
should be combined into love.error(msg, release)
(or love.errorhandler
, or something), with "release" being whether the game is in release mode or not.
I assume that error handilng may have enough similarities in release mode and non-release mode to have the same function with an argument. Perhaps the error messages are the same, perhaps they prepare the error message the same but display it differently, etc.
I also assume (and I'm not so sure of this), that checking for release mode is something that a lover would almost only want to do in the error callback. If checking for release mode is something which a lover would want to do elsewhere as well, then they would be checking the global variable love._release
, and having this as an argument to the error callback would be redundant.
Speaking of arguments-to-callbacks-made-redundant-by-global-variables, perhaps the command-line arguments givent to love.load
are a bit redundant and presumptuous because of the global variable arg
? If there is already a global variable, what's the point of the argument, and why for love.load
, perhaps I want command-line arguments to change the window dimensions in conf.lua
, or to toggle debug info in love.draw
. And if I only knew that it was an argument to love.load
, I'd probably assume that it wasn't a global variable, and therefore be confused as to how to use it in conf.lua
. Is the global variable arg
documented by the way?
INHALE!
So, in summary:
- Should
love.focus
be split into two functions? - I'm still thinking
love.(release)errhand
should be something likelove.error(msg, release)
. - Consider removing the
arg
argument fromlove.load
. - I like lowercase callbacks for some reason. :P
Original comment by Minh Ngo (Bitbucket: mngo, GitHub: mngo).
I think adding camel cases to the callbacks would be consistent with the rest of the API. Everything else has function names with camel cases! Although keeping the names lowercase would imply that they're callbacks.
I'm opposed to shortening names and would rather keep as much complete words as possible. I like that the devs changed the event names to full words as well. Function name shortening should be left to the individual like the popular lg = love.graphics.
With regards to love.errhand
and love.releaseerrhand
, I agree that they should be combined into one function like love.errorhandler(msg,release)
with release as a boolean.
With regards to all the joystick callbacks added (love.joystickaxismoved, love.joystickballmoved, and love.joystickhatmoved), I think they should all be combined into love.joystickinputted(joystick,type,...)
. The ...
are additional parameters according to the type
string (button,hat,axis,ball).
Original comment by Sasha Szpakowski (Bitbucket: slime73, GitHub: slime73).
Perhaps event callbacks could have an "on" prefix, and reside in their respective modules, e.g. love.joystick.onPress
, love.graphics.onFocus
, etc. (past-tense works as well).
I'm not sure how love.load
/love.update
/love.draw
would fit into that new scheme though, and it still has potential for some awkward names (love.joystick.onAxis
? love.joystick.onAxisMoved
?)
It might also present a problem if the modules aren't loaded when the callback functions are declared (confusing errors! yay!)
Original comment by Robin Wellner (Bitbucket: gvx, GitHub: gvx).
I disagree on the love.errorhandler(msg, release)
, because errhand
and releaseerrhand
have radically different purposes, even though they are called in similar situations:
errhand
has the purpose of delivering debugging information as fast as possible to either the lover who made the game or a playtester (who is often a lover as well).
releaseerrhand
has the purpose to explain something (not what) went wrong, and who to contact so it'll get fixed. They might see some debug information as well, but they're not expected to understand it or do anything with it except pass it on to the lover who made the game.
Original comment by Sasha Szpakowski (Bitbucket: slime73, GitHub: slime73).
In practice, the two end up being pretty much identical in most cases. It usually makes much more sense for game creators to have the error message and traceback displayed to end users than to omit them.
Original comment by Landon “Karai” Manning (Bitbucket: karai17, GitHub: karai17).
After discussing this with Sasha on IRC, I think we both agree on the following convention:
#!lua
-- Run cycle, not "events" per se
love.load
love.draw
love.update
-- System events
love.onFocus
love.onJoystickPressed
love.onJoystickReleased
love.onKeyPressed
love.onKeyReleased
love.onMousePressed
love.onMouseReleased
love.onQuit
Original comment by Sasha Szpakowski (Bitbucket: slime73, GitHub: slime73).
Yeah, although I think whatever happens, the callbacks should have consistent tenses. If we have love.onFocus
then we should have love.onJoystickPress
rather than love.onJoystickPressed
.
Original comment by Landon “Karai” Manning (Bitbucket: karai17, GitHub: karai17).
That's a fair point, too.
Original comment by Landon “Karai” Manning (Bitbucket: karai17, GitHub: karai17).
My vote is for present tense, not past tense.
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
love.onFocus
wouldn't really be on focus though... it would be more like "on change of focus", right? love.onFocusRecieved
and love.onFocusLost
could resolve this.
Would the event names also change to have the "on" prefix?
What about love.joystickaxis
etc.? love.onJoystickAxisMoved
I guess? In this case, it isn't really necessarily a more readable name though.
Regarding combining the error callbacks, even though they have different purposes, do they have that much different code in them? A lot of the default error handlers seem to be copy and pasted from one to the other.
But, I'm not sure if release mode is actually an elegant way to change the error message and save directory (#579).
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
Here's what I'm thinking currently:
-
Callbacks should be easily distinguishable from other functions. Simply not having them in modules achieves this.
-
I don't think it's necessary to differentiate the system events from the run cycle events by name. I'm not such a fan of the "on" prefix.
-
Having camelCase and present tense is pretty cool. It's consistent, after all.
-
I don't think there is another name which makes as much sense for the error function as
love.error
. After all, you can say "The love.x callback handles the "x" event" meaningfully for everything else.
What do people think of these?
#!lua
love.load
love.draw
love.update
love.quit
love.error
love.windowFocus
love.windowBlur
love.windowMouseOn
love.windowMouseOff
love.windowResize
love.joystickPress
love.joystickRelease
love.joystickHat
love.joystickAxis
love.keyPress
love.keyRelease
love.mousePress
love.mouseRelease
Original comment by Minh Ngo (Bitbucket: mngo, GitHub: mngo).
Thinking about it again, the current naming convention is fine as it makes functions stand out as overridable. I'd be pretty OK with adding present tense, but adding "on" would make it superfluous
Original comment by Anders Ruud (Bitbucket: rude, GitHub: rude).
A tense-consistent version of karai17's is a pretty good suggestion. However, what looks more correct than anything else is something like what slime (sometimes incorrectly referred to as "Sasha") suggested:
#!lua
-- Non-events. Thus no "on" prefix. We are not
-- notifying the game about something which happened,
-- we are demanding that something takes place.
love.load
love.draw
love.update
-- Events
love.window.onFocusReceived
love.joystick.onJoystickPressed
love.joystick.onJoystickReleased
-- "Key" and "Button" could potentially be dropped.
love.keyboard.onKeyPressed
love.keyboard.onKeyReleased
love.mouse.onButtonPressed
love.mouse.onButtonReleased
love.window.onClosed -- Alternative to onQuit?
As you can see I would argue for a past tense version, because in most cases at least, the thing which we are describing has already happened.
The only "problem" is that this would establish a new pattern of operation (POO) in LÖVE: the events originate from love.event, but are forwarded to special user-defined functions residing in the tables of other modules, presumably by love.run. I think this is OK. It's certainly not that much worse than forwarding to love.whatevereventtookplace.
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
Is the quit event dependent on the window module? If not, perhaps it shouldn't be in that module, lest someone want to do something on exit but not load the window module.
Also, I'm wondering where the error callback would go? Edit: And what would it be called? I guess onErrored
would be consistent, although it's not so pleasant grammatically.
Original comment by Landon “Karai” Manning (Bitbucket: karai17, GitHub: karai17).
@rude the only problem with the way you have set up, and I think slime mentioned this at some point, is if you write code for all modules (keyboard, joystick, etc) but dynamically load them, then your code will crash (wtf is love.joystick? BSOL) unless love.joystick is loaded. This disallows slimming of unneeded modules, or pre-writing code.
On the other hand, if we leave them all as events, then we can code them up and dynamically use them when needed.
Original comment by Sasha Szpakowski (Bitbucket: slime73, GitHub: slime73).
There might be some xtreme metatable fudgery that could be done to make it not error if a callback function is written for a module that isn't loaded, but... I don't think I want that kind of really hackish behaviour. :p
Original comment by Landon “Karai” Manning (Bitbucket: karai17, GitHub: karai17).
Well, if these are all events, why not just leave them in love.event and maybe add an alias to them from the other modules for semantic purposes?
love.joystick.onPressed = love.event.onButtonPressed
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
I'm unsure about the benefit of having callbacks in modules. I guess there's "disabling a module also disables the callback", but, like, I don't think anyone will disable the joystick module and then wonder why joystick callbacks aren't being called. :P
It's more verbose, which makes it harder and take more time to read, contrast love.keyboard.onKeyPressed
with love.keyPressed
.
I also think it makes the distinction between callbacks and functions-that-you-call less visually and conceptually clear.
Currently it's "Functions which start with love.
are callbacks and functions which start with love.module.
are functions you call".
If the callbacks were in modules, it would be "Functions which start with "on" are callbacks, as well as love.run
which also calls love.load
and love.update
and love.draw
, and functions in modules not starting with "on" are functions you call".
Original comment by Anders Ruud (Bitbucket: rude, GitHub: rude).
Yeah, I'm not sure I like the in-module callbacks myself when I actually have to use them in code.
#!lua
function love.keyboard.onKeyPressed(key)
-- Stuff.
end
Meh.
If not in the modules, then I also suggest this:
#!lua
-- Present tense, because these (non-)events are taking
-- place *now*.
love.load
love.draw
love.update
love.quit
love.err
-- Past tense, because these events already happened.
love.keyPressed
love.keyReleased
love.mousePressed
love.mouseReleased
love.windowFocused
love.windowBlurred
love.windowResized
love.joystickPressed
love.joystickReleased
love.joystickAxisMoved
love.joystickHatWTFed -- Because WTF is a joystick hat.
-- etc
So in general love.nounVerbed
for events which already happened.
I actually wanted an "on" prefix on the callbacks:
#!lua
love.onKeyPressed
love.onKeyReleased
-- etc
But then it would also "require" a prefix for load
, update
and draw
, for consistency:
#!lua
function love.onLoad()
end
function love.onUpdate()
end
function love.onDraw()
end
And that is clearly much uglier than the "trademark":
#!lua
function love.load()
end
function love.update()
end
function love.draw()
end
Original comment by Landon “Karai” Manning (Bitbucket: karai17, GitHub: karai17).
Differentiating between events and non events could allow for some leeway in naming, no? Since load/update/draw aren't events, they are things that just happen, it could be read "this is happening, do it now", but with events, "on" is appropriate because it can be read "WHEN this happens, do this".
Original comment by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).
Hmm, but 'onPoop' also kind of maybe suggests it registers something to happen when a poop occurs, not that it is the thing called when that happens. Like: love, on Poop, do this. Yet it's not a command, it's something you create, which is.. weird.
Original comment by Sasha Szpakowski (Bitbucket: slime73, GitHub: slime73).
Pfft, just go the Cocoa route:
love.windowDidBecomeVisible
I kid, I kid... :P
Original comment by Anders Ruud (Bitbucket: rude, GitHub: rude).
karai17: I thought so at first too, but then I discovered that it's perfectly logical to prefix "on" for load
, update
, draw
, error
and quit
(LUDEQ
) too. The fact that they are taking place now does not really free them from having a prefix if the other callbacks have it, it only allows us to use the present tense on those names, so we would not need onUpdated
, but we would need onUpdate
, in my opinion.
At least to me onUpdate
looks perfectly correct (in principle), and suggests that something else should happen while an update is happening.
But I don't like how onLUDEQ
looks in the code, and I don't think anyone else does either.
Original comment by Landon “Karai” Manning (Bitbucket: karai17, GitHub: karai17).
Yeah I'm honestly just trying to rationalize it away since onLUDEQ is ugly (though technically correct). I'm not really sure what the best course of action is at this point but I think we all agree that onLUDEQ is a no-go, on onEvent is still desirable as an outcome of this discussion.
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
I think love.verb
* for LUDTEQ** and love.nounVerbed
for all else might be quite nice.
However, are error
and quit
actually happening now, or have they already happened and now they're being handled?
* "error" is totally a verb now.
** Including "threadError".
#!lua
love.load
love.update
love.draw
love.error
love.threadError
love.quit
love.gamepadAxisMoved
love.gamepadPressed
love.gamepadReleased
love.joystickAxisMoved
love.joystickHatMoved
love.joystickPressed
love.joystickReleased
love.joystickAdded
love.joystickRemoved
love.keyPressed
love.keyReleased
love.mousePressed
love.mouseReleased
love.textEdited
love.textInputted
love.windowFocusGained
love.windowFocusLost
love.windowMouseFocusGained
love.windowMouseFocusLost
love.windowResized
love.windowVisibilityGained
love.windowVisibilityLost
(love.windowVisibilityGained
is exactly the same length as love.windowDidBecomeVisible
. :D)
And here are some names which make no sense. Which I kind of like.
#!lua
love.load
love.update
love.draw
love.error
love.threadError
love.quit
love.gamepadAxis
love.gamepadDown
love.gamepadUp
love.joystickAxis
love.joystickHat
love.joystickDown
love.joystickUp
love.joystickAdded
love.joystickRemoved
love.keyDown
love.keyUp
love.mouseDown
love.mouseUp
love.textEdit
love.textInput
love.focus
love.unfocus
love.mouseOver
love.mouseOff
love.resize
love.visible
love.invisible
Original comment by Landon “Karai” Manning (Bitbucket: karai17, GitHub: karai17).
I'll verb YOUR noun. ;)
love.textInputted and love.textEdited seem a bit of a stretch, but the rest of them read well enough.
Original comment by Anders Ruud (Bitbucket: rude, GitHub: rude).
love.textEdited seems OK, but love.textInputted sounds quite bad to me.
"Inputted" and "input" are both valid past tense forms, so perhaps we should use love.textInput instead.
Original comment by Sasha Szpakowski (Bitbucket: slime73, GitHub: slime73).
However, are
error
andquit
actually happening now, or have they already happened and now they're being handled?
quit
is triggered after the game has been requested to quit (either via love.event.quit()
or the user pressing the close button on the window.) error
is triggered after an error occurs.