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

Massive refactor: split codegen into JSON and domain models #573

Merged
merged 10 commits into from
Jan 21, 2024

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Jan 21, 2024

A lot of issues in godot-codegen have arisen because all business logic (analyzing symbols, generating code, ...) was operating on the raw JSON models from the extension API. While this worked well in the beginning, it caused more and more issues over time and became a scalability hazard:

  • The JSON models had inconvenient data structures (e.g. Option<Vec<T>> instead of Vec<T>).
  • Derived information needed to be constantly recomputed or stored elsewhere.
  • Several bugs occurred due to mixing our Rust representation with Godot's original one, for example regarding names.
  • The logic to map from JSON to domain model was all over the place -- centralizing it makes things much easier.

This is the foundation for more codegen cleanups and is direly needed for some of the features I plan to implement soon.

@Bromeon Bromeon added quality-of-life No new functionality, but improves ergonomics/internals c: tooling CI, automation, tools labels Jan 21, 2024
@Bromeon Bromeon enabled auto-merge January 21, 2024 13:42
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-573

@Bromeon Bromeon added this pull request to the merge queue Jan 21, 2024
Merged via the queue into master with commit 6143ed4 Jan 21, 2024
@Bromeon Bromeon deleted the qol/codegen-split-json branch January 21, 2024 13:53
@Bromeon Bromeon removed the c: tooling CI, automation, tools label Jan 21, 2024
@Bromeon Bromeon mentioned this pull request Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quality-of-life No new functionality, but improves ergonomics/internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants