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

Add graphics support for building-hacks #4380

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

Conversation

warmist
Copy link
Member

@warmist warmist commented Mar 17, 2024

This intends to reenable building-hacks namely machine workshops with animations.

Things to consider:

  • general "is this useful to anyone else"?
  • doc update not done yet
  • (?) redo the api as it sucks. Lua part simplifies the api a little...
  • how do you get tile ids (ala raws) in lua (i.e. how to make the api work?)
  • getImpassableOccupancy fix still needed?
  • canBeRoomSubset thingy for interesting room manipulations, but not sure about usecases
  • not very kosher use of random reference for machine power tracking...
  • updateAction - probably could be removed and replaced with a lua on tick update or sth...

Edit: less theoretical musings, more "works for now" logic.

  • make stuff automagically work from raws moved to a lua only module (different pull req later)

plugins/building-hacks.cpp Outdated Show resolved Hide resolved
plugins/building-hacks.cpp Outdated Show resolved Hide resolved
@myk002
Copy link
Member

myk002 commented Mar 17, 2024

Can the plugin automatically enable itself if it detects that it is used in the raws, the same way that add-spatter does? That way the player won't have to manually enable it to get things working.

  • general "is this useful to anyone else"?

as useful as add-spatter, which I'm also happy to include

  • how do you get tile ids (ala raws) in lua (i.e. how to make the api work?)

it depends which tile ids you're getting. will it be a vanilla texpos or one added by DFHack?

@warmist
Copy link
Member Author

warmist commented Mar 17, 2024

Can the plugin automatically enable itself if it detects that it is used in the raws, the same way that add-spatter does? That way the player won't have to manually enable it to get things working.

It depends: for basic functionality it should be possible. It can be split in a few different things that this plugin does:

  1. Impassable fix - this might be already in df? Need to check. Basically fixes that workshop impassible tiles would allow water through.
  2. Generate power - unlikely that it's useful. Usually need some sort of "when fuel exists/etc" and probably used from lua
  3. Consume power - probably useful straight from raws, but it needs an argument (i.e. how much power do you need)
  4. animation - have no idea how to do it from raws
  5. canBeRoomSubset - ugh...
  • how do you get tile ids (ala raws) in lua (i.e. how to make the api work?)

it depends which tile ids you're getting. will it be a vanilla texpos or one added by DFHack?

It's vanilla texpos but e.g. you do:

graphics_dragon_engine

[OBJECT:GRAPHICS]

[TILE_GRAPHICS:DRAGON_ENGINE_TILES:0:0:WORKSHOP_CUSTOM:DRAGON_ENGINE_N:3:0:0]
[TILE_GRAPHICS:DRAGON_ENGINE_TILES:0:1:WORKSHOP_CUSTOM:DRAGON_ENGINE_N:3:0:1]
[TILE_GRAPHICS:DRAGON_ENGINE_TILES:0:2:WORKSHOP_CUSTOM:DRAGON_ENGINE_N:3:0:2]
[TILE_GRAPHICS:DRAGON_ENGINE_TILES:0:3:WORKSHOP_CUSTOM:DRAGON_ENGINE_N:3:0:3]
[TILE_GRAPHICS:DRAGON_ENGINE_TILES:0:4:WORKSHOP_CUSTOM:DRAGON_ENGINE_N:3:0:4]

and then you need yet another tile for animated part. Or you need to do something similar in the lua part.

@myk002
Copy link
Member

myk002 commented Mar 17, 2024

add_spatter looks for SPATTER_ADD_ in any reaction. Could this look for a similar token prefix across the loaded building raws?

for looking up the graphics tile from a loaded page, I think you'd use dfhack.screen.findGraphicsTile(pagename,x,y)
that's slow, though, so you'd probably want to cache it and wipe your cache on a map load event.

@warmist warmist force-pushed the building_hacks_graphics branch 2 times, most recently from 714c6ce to 0c5c753 Compare March 18, 2024 15:35
@warmist
Copy link
Member Author

warmist commented Mar 18, 2024

So api is still terrible but less so. The rest of stuff doesn't matter that much, so i think i'll add (or at least try to add) automagical way of this loading stuff from raws and auto-enabling itself. In perfect case having a line [DFHACK_MACHINE:25] would make any workshop to have power drain of 25 to work. This ofc is not perfect (e.g. how about interface, what if you want to provide power, etc...) but it would lower cost of entry for a lot of modders.

Another issue that plagues this module for many years: buildings need to built in specific order to connect to machines...

plugins/building-hacks.cpp Outdated Show resolved Hide resolved
plugins/building-hacks.cpp Outdated Show resolved Hide resolved
plugins/lua/building-hacks.lua Outdated Show resolved Hide resolved
@warmist
Copy link
Member Author

warmist commented Mar 24, 2024

This is now relatively done. Needs more testing.

The "make it work from raws" is a separate project and probably lua only.

@warmist warmist changed the title [WIP] Add graphics support for building-hacks Add graphics support for building-hacks Mar 29, 2024
@warmist
Copy link
Member Author

warmist commented Mar 29, 2024

Ok, this is as done as I currently can make it be.
Spoke too soon. Some issue remains with 0 or 1 based building tile indexing?
Ok NOW it's done 👯

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.

reviewed docs and lifecycle code; will review business logic once some questions are answered.

  • could you add a changelog entry for the update?
  • could you update docs/plugins/building-hacks.rst and replace the "unavailable" tag with "fort gameplay buildings". Maybe "adventure fort gameplay buildings" if it's intended to work in adventure mode too.
  • we need a full example of the RAWs and scripts for how to make this all work. It could be added to the modding guide as a case study. You could link to the modding guide from both the Lua API docs and buiding-hacks.rst.

docs/dev/Lua API.rst Outdated Show resolved Hide resolved
plugins/building-hacks.cpp Outdated Show resolved Hide resolved
plugins/building-hacks.cpp Show resolved Hide resolved
plugins/building-hacks.cpp Show resolved Hide resolved
plugins/building-hacks.cpp Outdated Show resolved Hide resolved
docs/dev/Lua API.rst Show resolved Hide resolved
docs/dev/Lua API.rst Outdated Show resolved Hide resolved
:make_graphics_too: replace same tiles in graphics mode with tiles from vanilla df mechanism
:frame_skip: How many ticks to display one frame. If set to negative number (or skipped) frames
are synchronized with machine animation.
:gear_tiles: Optional array of 2 or 4 indexes. First two define ascii tiles and next two graphics tiles
Copy link
Member

Choose a reason for hiding this comment

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

is this for overriding the default gear graphics? If so, that should be communicated.

docs/dev/Lua API.rst Outdated Show resolved Hide resolved
------

This module exports two events. However only one is documented here and is intended to be used directly. To use
``onUpdateAction`` instead call ``setOnUpdate`` function.
Copy link
Member

Choose a reason for hiding this comment

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

why does onUpdateAction exist as a Lua event, then?

Copy link
Member Author

Choose a reason for hiding this comment

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

The plugin lua part still uses that event (i.e. setOnUpdate function)

Copy link
Member

Choose a reason for hiding this comment

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

I'm not convinced this is sufficient reasoning. Why do you need a lua event to communicate with your own Lua layer? Why can't you call the Lua functions directly?

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.

needs changelog entry

I added some code examples in my comments to show what my understanding is of how these functions are to be used. If the examples are wrong, then I didn't understand something important.


:name:
custom workshop id e.g. ``SOAPMAKER``
Set workshop to be included in zones (such as bedroom or inn).
Copy link
Member

Choose a reason for hiding this comment

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

Would it be a good idea to always allow the building to be ownable? If in zone, and zone owned, then building owned?

plugin export a function it's recommended to use lua decorated function.
This plugin extends DF workshops to support custom powered buildings.

.. note:: when using numeric ids for workshops be aware that those id can change between worlds
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
.. note:: when using numeric ids for workshops be aware that those id can change between worlds
.. note::
When using numeric ids for workshop types, be aware that those ids can change
between worlds, depending on what other custom types exist in the raws for that
world.

``registerBuilding(table)`` where table must contain name, as a workshop raw name, the rest are optional:
* ``setOwnableBuilding(workshop_type)``

Set workshop to be included in zones (such as bedroom or inn).
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
Set workshop to be included in zones (such as bedroom or inn).
Set workshop to be included in zones (such as a bedroom or tavern).


Set workshop to be included in zones (such as bedroom or inn).

:workshop_type: custom workshop string id e.g. ``SOAPMAKER`` or numeric id
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
:workshop_type: custom workshop string id e.g. ``SOAPMAKER`` or numeric id
:workshop_type: custom workshop string id, e.g. ``SOAPMAKER`` or numeric id


Set workshop non walkable tiles to also block liquids (i.e. water and magma).

:workshop_type: custom workshop string id e.g. ``SOAPMAKER`` or numeric id
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
:workshop_type: custom workshop string id e.g. ``SOAPMAKER`` or numeric id
:workshop_type: custom workshop string id, e.g. ``SOAPMAKER`` or numeric id


``setPower(building,produced,consumed)`` sets current power production and consumption for a building.
Returns two number - produced and consumed power if building can be modified and returns nothing otherwise.
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
Returns two number - produced and consumed power if building can be modified and returns nothing otherwise.
If this building is of a type registered with building-hacks, returns values for
produced and consumed power. Otherwise, returns ``nil``.

Comment on lines +5856 to +5857
Sets current power production and consumption for a specific workshop building. Can be used to make buildings that
dynamically change power consumption and production.
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
Sets current power production and consumption for a specific workshop building. Can be used to make buildings that
dynamically change power consumption and production.
Dynamically sets current power production and consumption for a specific workshop
(which must be of a type registered with building-hacks).

Copy link
Member

Choose a reason for hiding this comment

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

I note that the order of consumed and produced power is in the opposite order compared to setPower. They should match.

------

This module exports two events. However only one is documented here and is intended to be used directly. To use
``onUpdateAction`` instead call ``setOnUpdate`` function.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not convinced this is sufficient reasoning. Why do you need a lua event to communicate with your own Lua layer? Why can't you call the Lua functions directly?

This module exports two events. However only one is documented here and is intended to be used directly. To use
``onUpdateAction`` instead call ``setOnUpdate`` function.

* ``onSetTriggerState(workshop,state)``
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
* ``onSetTriggerState(workshop,state)``
* ``onSetTriggerState(workshop, state)``

frames={
{{x=0,y=0,42,7,0,0}}, --first frame, 1 changed tile
{{x=0,y=0,15,7,0,0}} -- second frame, same
local bld=require('plugins.building-hacks')
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
local bld=require('plugins.building-hacks')
local bhacks = require('plugins.building-hacks')

Copy link
Member

Choose a reason for hiding this comment

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

bld is consistently used throughout DFHack to refer to "current building". using the name to refer to the module would be confusing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants