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

Improve luaengine comments (fixes and updates) #5014

Merged
merged 3 commits into from May 9, 2019

Conversation

Projects
None yet
2 participants
@vadosnaprimer
Copy link
Contributor

commented May 8, 2019

  • This is not directly related to outputting lua help (#3000), but if those comments are accurate, it'd be a a step towards that goal. Therefore, I'd like this to be reviewed.
  • Only touching comments, not code
  • Tested everything I could get to work (watchpoint and thread libraries completely refused to work and kept crashing or giving errors)
  • Fix syntax mismatch
  • Add missing functionality (don't know where to add address_map_entry comments not to break actual code; also don't know how to use input:seq_poll*)
  • Reorder and group methods and fields together
  • Call out libraries by object type
  • Include full commands to access library objects (short commands relied on other commands which were scattered around the file, requiring constantly looking up every part of the command)
  • Mention keys and values of table entries (except bplist and wplist since I don't know how to inject comments nicely there)

@rb6502 rb6502 requested a review from cracyc May 8, 2019

@cracyc

This comment has been minimized.

Copy link
Member

commented May 9, 2019

Tested everything I could get to work (watchpoint and thread libraries completely refused to work and kept crashing or giving errors)

The console plugin uses the thread functions, it's rather basic right now but more functionality can be added if needed. The gdbstub uses the watchpoints, both it and the breakpoints require the debugger to be enabled (I thought that was mentioned in the comments but apparently not).

also don't know how to use input:seq_poll*

This is used in the cheat plugin like

local function hkcbfunc(cheat)
	local input = manager:machine():input()
	manager:machine():popmessage(_("Press button for hotkey or wait to clear"))
	manager:machine():video():frame_update(true)
	input:seq_poll_start("switch")
	local time = os.clock()
	while (not input:seq_poll()) and (os.clock() < time + 1) do end
	cheat.hotkeys = {pressed = false, keys = input:seq_poll_final()}
	manager:machine():popmessage()
	manager:machine():video():frame_update(true)
end

* machine:popmessage(str) - print str as popup
I neglected to fix this but if str is omitted then any displayed popup message is cleared.

* space:write_*(addr)
These should be space:write_*(addr, data)

luaengine comments: add missing info:
emu.thread()
machine:popmessage()
watch/breakpoints
debugger presence note
val for write functions
input:seq_poll*
fix typo in memory_share library

emu.register_callback(callback, name) still TODO
@vadosnaprimer

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2019

@cracyc thanks, I figured things out, just one last todo is how to use emu.register_callback(callback, name). I see it in a couple scripts but can't understand how it's triggered.

@cracyc

This comment has been minimized.

Copy link
Member

commented May 9, 2019

It's just a generic named callback. Once registered it can be called with lua_engine::call_plugin(name, in, out) where in and out are the parameters passed to and returned from the plugin. It's use mostly by the data plugin for the text displayed in the ui and examples are in datmenu.cpp and selmenu.cpp.

@vadosnaprimer

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2019

Got it. Would you mind if I include a new luaengine function into this PR, emu.step()? ui.single_step is settable, but if set to true it can only trigger a pause, there's no way to advance a frame from lua.

@cracyc

This comment has been minimized.

Copy link
Member

commented May 9, 2019

That's fine.

@vadosnaprimer

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2019

Okay I guess I'm done.

@cracyc cracyc merged commit 5c527ad into mamedev:master May 9, 2019

0 of 3 checks passed

continuous-integration/tea oops, something went wrong
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.