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

persist liquidSources table #920

Merged
merged 17 commits into from
Jan 6, 2024
Merged

Conversation

Heneman
Copy link
Contributor

@Heneman Heneman commented Jan 4, 2024

@Heneman Heneman changed the title persist liquidSources table DRAFT: persist liquidSources table Jan 4, 2024
source.lua Outdated Show resolved Hide resolved
@myk002 myk002 marked this pull request as draft January 4, 2024 04:17
@myk002 myk002 changed the title DRAFT: persist liquidSources table persist liquidSources table Jan 4, 2024
@Heneman Heneman closed this Jan 4, 2024
@Heneman Heneman reopened this Jan 4, 2024
source.lua Outdated Show resolved Hide resolved
@Heneman Heneman marked this pull request as ready for review January 5, 2024 03:53
@Heneman Heneman marked this pull request as draft January 5, 2024 04:01
@Heneman
Copy link
Contributor Author

Heneman commented Jan 5, 2024

persisting but sources will stack instead of overwrite if one is added to a tile that already has one. i imagine we don't want more than once source on a tile so i can add that in, but persistence is currently functional 🎉

@Heneman Heneman marked this pull request as ready for review January 5, 2024 06:13
@Heneman
Copy link
Contributor Author

Heneman commented Jan 5, 2024

It's working! IT'S WORKING!

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.

looks like all the major pieces are in place! great job! just needs some cleanup. Thank you for getting this done! I can do another review pass once this is cleaned up.

source.lua Outdated Show resolved Hide resolved
source.lua Outdated Show resolved Hide resolved
source.lua Outdated Show resolved Hide resolved
source.lua Outdated Show resolved Hide resolved
source.lua Outdated Show resolved Hide resolved
source.lua Outdated Show resolved Hide resolved
source.lua Outdated Show resolved Hide resolved
source.lua Show resolved Hide resolved
source.lua Show resolved Hide resolved
source.lua Outdated Show resolved Hide resolved
@Heneman
Copy link
Contributor Author

Heneman commented Jan 5, 2024

pre-commit.ci autofix

@Heneman
Copy link
Contributor Author

Heneman commented Jan 5, 2024

o.O

I thought I'd be triggering a cleanup of the trailing whitespace that the precommit check was showing failed, I didn't intend for a build to be triggered. Apologies for any mess that caused 😬

@myk002
Copy link
Member

myk002 commented Jan 5, 2024

I didn't intend for a build to be triggered. Apologies for any mess that caused

A build is supposed to be triggered after every change -- that's how we detect potential failures before we merge them. Not a mess. Working as intended.

source.lua Outdated Show resolved Hide resolved
source.lua Show resolved Hide resolved
source.lua Outdated Show resolved Hide resolved
source.lua Show resolved Hide resolved
source.lua Outdated Show resolved Hide resolved
source.lua Outdated Show resolved Hide resolved
source.lua Show resolved Hide resolved
source.lua Outdated Show resolved Hide resolved
source.lua Outdated Show resolved Hide resolved
source.lua Show resolved Hide resolved
@Heneman
Copy link
Contributor Author

Heneman commented Jan 5, 2024

Should be ready for 👀 again! The rearrangement of functions and some cleanup means some of the comments aren't resolving, but I think I got everything that was active

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.

looks good! remember to add a line to the Misc Improvements section (under the "Future" version header) of changelog.txt. Something like:

- `source`: now saves its state with the fort

source.lua Outdated Show resolved Hide resolved
source.lua Show resolved Hide resolved
@myk002
Copy link
Member

myk002 commented Jan 6, 2024

Great job with this! Thank you for working with me through all the comments!

@myk002 myk002 merged commit a3e0050 into DFHack:master Jan 6, 2024
10 checks passed
@Heneman
Copy link
Contributor Author

Heneman commented Jan 6, 2024

Happy to help!

Headed back to my failed megaproject with my newfound magic powers:

so long

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.

Liquid generation from source tiles doesn't persist between loads
2 participants