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

Update quantum.lua to v50 #834

Merged
merged 4 commits into from
Apr 27, 2024
Merged

Conversation

MarioWizard119
Copy link
Contributor

WHAT THIS PR DO
Updates gui/quantum to be able to be used on v50 saves. A bit jank, but mostly functional

WHAT THIS PR DON'T DO, BUT PROLLY NEED TO BE DONE (BE IT BY ME OR SOMEONE FAR SMARTER)
Unfortunately I couldn't get the script to automatically order a minecart if there wasn't one. From what I could gather from the old gui/quantum file, that too was handled by quickfort. That could probably be fixed later once I figure out how to do it, but a stopgap measure it to just warn that one will need to be made beforehand or after.

Also could probably use a cleanup or optimization, I'm extremely new to this, and this is the first actual project I've contributed to (I don't count my Space Station 13 days. Writing an entire goddamn essay to explain why changing two variables won't destroy the game's balance isn't coding, It's the psychological equivalent of a root beer float, but with mint chocolate chip ice cream and orange Fanta instead), so feel free to make/have me make changes to it to adhere to community standards. And also, for some reason I couldn't get the script to intercept left clicks on the map whatsoever so I had to resort to a janky hotkey workaround, but it's functional.

copy pasted from the text doc, hopefully everything copied over correctly
gui/quantum.lua Outdated
focus_path='quantum',
sidebar_mode=df.ui_sidebar_mode.LookAround,
defocusable=true
Copy link
Member

Choose a reason for hiding this comment

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

defocusable is true by default, so you can remove that one. I also suggest you follow the example of gui/blueprint and set

pass_movement_keys=true,
pass_mouse_clicks=false,

gui/quantum.lua Outdated
@@ -275,80 +225,57 @@ local function order_minecart(pos)
quickfort_orders.enqueue_additional_order(quickfort_ctx, 'wooden minecart')
quickfort_orders.create_orders(quickfort_ctx)
end
--Key Input
function QuantumUI:onInput(keys)
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, this will break the ZScreen stack (that is, input will not be propagated properly). Try splitting out the Window widget into its own class and attaching input processing code there. Check out the example architecture in the docs here: https://docs.dfhack.org/en/latest/docs/dev/Lua%20API.html#zscreen-class

gui/quantum.lua Outdated
label='Place QSP under cursor',
on_activate=self:callback('commit')
},
widgets.Panel{autoarrange_subviews=true,
Copy link
Member

Choose a reason for hiding this comment

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

indents should be four spaces

gui/quantum.lua Outdated
function QuantumUI:get_help_text()
if not self.feeder then
if not dfhack.gui.getSelectedStockpile() then
return 'Please select the feeder stockpile with the cursor or mouse.'
Copy link
Member

Choose a reason for hiding this comment

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

there isn't really a cursor option for this anymore. directions should be to use the mouse

gui/quantum.lua Outdated
name = 'Unnamed QSP'
end
placed_output = output_type..'{quantum=true}'
placed_trackstop = trackstop_dir..'{take_from='..input_id..' route="'..name..'"}'
Copy link
Member

Choose a reason for hiding this comment

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

the trackstop itself should get a name, something like "$name dumper"

setting the route property to a name that already exists will attach to an existing route, which is never what you want for a qsp. I suggest either enforcing a unique name or just not naming the route

gui/quantum.lua Outdated
Comment on lines 210 to 214
data={
[0]={
[0]={[0]=placed_trackstop}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
data={
[0]={
[0]={[0]=placed_trackstop}
}
}
data=placed_trackstop

gui/quantum.lua Outdated
Comment on lines 218 to 219
message= 'QSP Placed!'
dialogs.MessageBox{text=message:wrap(70)}:show()
Copy link
Member

Choose a reason for hiding this comment

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

does this need to be a popup? could we perhaps instead write it to the gui/quantum window panel?

gui/quantum.lua Outdated
then
name = 'Unnamed QSP'
end
placed_output = output_type..'{quantum=true}'
Copy link
Member

Choose a reason for hiding this comment

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

should be of the form:
ry{name="$name quantum" quantum=true}:+all
or
c{name="$name quantum" quantum=true}:+all

@myk002
Copy link
Member

myk002 commented Oct 5, 2023

Unfortunately I couldn't get the script to automatically order a minecart if there wasn't one. From what I could gather from the old gui/quantum file, that too was handled by quickfort.

quickfort can still be responsible for ordering the minecart. that's what the quickfort orders command is for.

Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

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

could you possibly fix the formatting? See summary of errors here: https://results.pre-commit.ci/run/github/54066125/1699332446.rdyxy0-8R_eBoKIIwgdL8w

@MarioWizard119
Copy link
Contributor Author

pre-commit.ci autofix

gui/quantum.lua Outdated
Comment on lines 35 to 37
widgets.Label{
text='Quantum'
},
Copy link
Member

Choose a reason for hiding this comment

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

"Quantum" is now in the title bar and this Label can be removed

gui/quantum.lua Outdated
QuantumUI = defclass(QuantumUI, guidm.MenuOverlay)

-- UI Layout
QuantumUI = defclass(QuantumUI, gui.ZScreen)
Copy link
Member

Choose a reason for hiding this comment

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

Try not to mix the functions between two classes. It makes the logic of the file much harder to follow. Put the ZScreen subclass at the bottom and the Window subclass (and all other business logic) at the top

gui/quantum.lua Outdated
main_panel:addviews{
widgets.Label{text='Quantum'},
function QuantumWin:init()
cart_count = #assign_minecarts.get_free_vehicles()
Copy link
Member

Choose a reason for hiding this comment

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

cart_count should be an instance variable, not a global. call it self.cart_count instead

gui/quantum.lua Outdated
Comment on lines 35 to 37
widgets.Label{
text='Quantum'
},
Copy link
Member

Choose a reason for hiding this comment

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

this is in the frame title now and doesn't need to be a label

gui/quantum.lua Outdated
Comment on lines 42 to 46
widgets.HotkeyLabel{
key='CUSTOM_Q',
label='Place QSP under cursor',
on_activate=self:callback('commit')
},
Copy link
Member

@myk002 myk002 Nov 8, 2023

Choose a reason for hiding this comment

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

I'd say don't bother with the keyboard cursor for now. Few people know how to use it and it complicates the UI. Go with mouse clicks, like gui/quantum and gui/blueprint and other map tools do

edit: I notice that later in the commit function, the keyboard cursor isn't even consulted for where to apply the blueprint. I think it is better if this tool were just mouse-only for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had left the keyboard option in cause there's a part o the community that gets cranky when there's not keyboard shortcuts for stuff. The Q key places it where the mouse cursor is, so I can see why it would be redundant, and I understand it not being worth it to cater to that small subsection of the community.

gui/quantum.lua Outdated
then
self:commit()
elseif
keys._MOUSE_L_DOWN and not self:getMouseFramePos()
Copy link
Member

Choose a reason for hiding this comment

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

should be _MOUSE_L, not _MOUSE_L_DOWN

gui/quantum.lua Outdated
if
cart_count == 0
then
dfhack.run_command('workorder "{\"amount_left\":1,\"amount_total\":1,\"frequency\":\"OneTime\",\"id\":6,\"is_active\":false,\"is_validated\":true,\"item_subtype\":\"ITEM_TOOL_MINECART\",\"job\":\"MakeTool\",\"material_category\":[\"wood\"]}"')
Copy link
Member

Choose a reason for hiding this comment

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

why are you ordering a minecart via workorder instead of the order_minecart function below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basic answer was it was what I could get to work. The order Minecart function was one of the things I couldn't seem to get to work properly before, but now that the class stuff's been sorted out I'll see what I can do about it. I chose work order specifically because it wasn't reliant on any external blueprint or work order file and I could use a json string in the script itself. I can change it if you want me to.

gui/quantum.lua Outdated

local tiles = {}
--UI Data Tables
qspOutputHor = {
Copy link
Member

Choose a reason for hiding this comment

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

these should all be declared local

gui/quantum.lua Outdated
dump_dir = self.subviews.dir:getOptionLabel()
allow_refuse = self.subviews.refuse:getOptionValue()
input_id = dfhack.gui.getSelectedStockpile().id
output_hor = qspOutputHor[dump_dir]
Copy link
Member

Choose a reason for hiding this comment

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

these should all be declared local

gui/quantum.lua Outdated
placed_output = output_type..'{name="'..name..'" quantum=true}:+all'
placed_trackstop = trackstop_dir..'{name="'..name..'" take_from='..input_id..'}'

quickfort.apply_blueprint{
Copy link
Member

Choose a reason for hiding this comment

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

this needs the error checking that was in the original code -- if the player has selected an invalid tile for the QSP, we should not blindly move forward

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was somewhat of a deliberate design choice. There are a few uses for a pseudo QSP where the output tile isn't valid to put a stockpile on, for example, dumping goblin corpses down a mass grave out of sight and out of mind, dumping ores down a huge shaft down to the magma sea and magma forges, and dumping iron minecarts onto a roller system to fill them with magma. I've even used quantum to make storm drains and water reactors via a dummy 1x1 input stockpile with no settings, since they both use the same mechanism of a dumping Minecart.

As for the error checking, wouldn't it be redundant? From what I could tell from the old version, the error checking went place trackstop (dry run), if dry placed entities = 0 then display error message, else place trackstop. What's stopping us from place trackstop, if placed entities = 0 then display error message? I could see it being necessary for the trackstop itself, rather than the output stockpile because regardless of the above niche uses or an actual QSP, neither will work without the trackstop, unless it causes problems in the backend I'm not aware of.

@myk002 myk002 merged commit aa50d82 into DFHack:master Apr 27, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants