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

Great Reorganization #824

Open
wants to merge 125 commits into
base: master
Choose a base branch
from
Open

Great Reorganization #824

wants to merge 125 commits into from

Conversation

quietust
Copy link
Member

@quietust quietust commented Feb 1, 2025

This PR reorganizes all of the structure definitions into new files corresponding to Toady's header filenames, with all types in the same order as Toady's.

Notable changes:

  • All structures, enums, and bitfields are now declared as top-level types
  • In a number of places, new <compound>s have been added, which may result in the "paths" to certain variables being different.
  • Several "fake" structure definitions (which we added ourselves in order to reduce duplication in places where Toady actually copy/pasted fields) have been removed, with their contents substituted directly into the places where they were previously referenced.

The goal of this change is to make the structure definitions more straightforward to update in the future.

I have not yet attempted to build DFHack against these updates, but I know that there are some changes which will require modifications to DFHack. The total number of codegen output files has increased from 2155 to 3284 so there's going to be a VERY long list of structure size changes to check, but most of them should just be "0 to NNNN" - all existing structures should stay the same size, so if there are any that grew or shrunk, it's possible I introduced an error somewhere along the way.

@quietust
Copy link
Member Author

quietust commented Feb 1, 2025

This has apparently revealed a bug in codegen: bitfields and enums with a base-type of ulong are not correctly translating that to unsigned long in the resulting header files, resulting in compiler errors.

I don't understand the codegen scripts well enough, so I have no idea how to fix this.

@myk002
Copy link
Member

myk002 commented Feb 2, 2025

df.region.xml and df.d_interface.xml fail the pre-commit check (trailing whitespace)

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.

I am in awe of this PR

Is the list of field name/nesting changes in Discord complete? How risky do you think this refactoring is?

My gut feeling is that we should:

  1. do a bug-fix-focused release of 51.04-r2, branched from 51.04-r1 and just including bugfix cherry picks
  2. merge this, update the dfhack and scripts repos accordingly, and merge a bunch of other new features
  3. do a beta release 51.04-r3rc1 (we can expect the new features to draw a good number of beta testers)
  4. iterate on the beta for a while
  5. release 51.04-r3 after a few weeks

df.d_init.xml Outdated Show resolved Hide resolved
@quietust quietust force-pushed the master branch 2 times, most recently from a971f1c to ff9c640 Compare February 8, 2025 01:04
* fix bug in codegen with enums/bitfields with base-type "ulong"
* fix item_tool_graphics_flag so it compiles (Toady's definition is already wrong)
* fix site_architecture_change (two anonymous unions with conflicting names)
* fix size of d_init_announcementst
* add missing histfig_body_state.NONE (from whereabouts_type)
* fix instance-vectors for newly-created handlers
* get rid of "dye_info" structure
* remove trailing whitespace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants