Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
…es from the catalog
tturocy
force-pushed
the
dev_catalog_gui
branch
from
September 10, 2026 11:19
8e7ffc5 to
a13dbd3
Compare
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed crash/UX issues in the new C++ manifest parsing/error handling and several catalog game description typos that should be fixed (with the GUI manifest regenerated) before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR integrates the curated games catalog into the GUI build/install so users can browse and open bundled catalog games from the welcome screen and File menu, while also reorganizing catalog assets under catalog/games and splitting the catalog docs into a separate Sphinx/ReadTheDocs project.
Changes:
- Add a GUI “Browse catalog” dialog and wire it into the welcome screen and File menu (“Open from Catalog…”).
- Restructure catalog packaging/build: move catalog content under
catalog/games, generate and ship a GUI-facingmanifest.json, and install the catalog alongside the GUI on all platforms. - Update documentation and CI/workflows to reflect the catalog split (separate RTD project) and new paths.
File summaries
| File | Description |
|---|---|
| src/pygambit/catalog.py | Prefer repo checkout catalog/games for dev/editable installs; update download links to new layout. |
| src/gui/welcome.h | Add welcome-screen event and UI members for catalog browsing. |
| src/gui/welcome.cc | Add “Browse catalog” button and open flow via the catalog browser dialog. |
| src/gui/menuconst.h | Add menu constant for “Open from Catalog…”. |
| src/gui/gameframe.h | Add handler declaration for opening from the catalog. |
| src/gui/gameframe.cc | Add File menu item and handler to launch the catalog browser dialog. |
| src/gui/dlcatalog.h | Introduce the catalog browser dialog UI contract. |
| src/gui/dlcatalog.cc | Implement the catalog browser dialog (search/filter/list/details + open). |
| src/gui/catalogdata.h | Add manifest-driven catalog entry model and loader APIs for GUI. |
| src/gui/catalogdata.cc | Implement catalog root discovery, manifest loading, and minimal JSON parsing. |
| src/gui/app.cc | Set app name early to stabilize wxStandardPaths-derived locations (catalog discovery). |
| setup.py | Package catalog/games into pygambit/catalog_data during Python build. |
| pyproject.toml | Add pybtex to doc extras; adjust pytest pythonpath/marker wording for new catalog build script. |
| MANIFEST.in | Limit sdist inclusion to catalog/games instead of whole catalog. |
| Makefile.am | Include new catalog make fragment; install catalog data for GUI builds; add new GUI sources and SRCDIR define. |
| doc/tutorials/interoperability_tutorials/gamut.ipynb | Update link target to new catalog generators page. |
| doc/tools.simpdiv.rst | Update example path to catalog/games/.... |
| doc/tools.logit.rst | Update example path to catalog/games/.... |
| doc/tools.liap.rst | Update example path to catalog/games/.... |
| doc/tools.lcp.rst | Update example path to catalog/games/.... |
| doc/tools.ipa.rst | Update example path to catalog/games/.... |
| doc/tools.hp.rst | Update example path to catalog/games/.... |
| doc/tools.gnm.rst | Update example path to catalog/games/.... |
| doc/tools.enumpure.rst | Update example path(s) to catalog/games/.... |
| doc/tools.enumpoly.rst | Update example path to catalog/games/.... |
| doc/tools.enummixed.rst | Update example path(s) to catalog/games/.... |
| doc/index.rst | Point “Catalog of games” card to external catalog docs; update toctree to catalog_generators. |
| doc/developer.rst | Add pointer to separate catalog developer guide; remove old developer catalog page link. |
| doc/developer.overview.rst | Replace internal catalog ref with external catalog documentation link. |
| doc/conf.py | Move bib style registration into shared doc/_bibstyle.py; simplify exclude patterns. |
| doc/catalog.csv | Add small CSV catalog list (doc/data artifact). |
| doc/catalog_generators.rst | Replace old catalog page with generator-focused page (OpenSpiel + GAMUT) and external catalog pointers. |
| doc/_bibstyle.py | Add shared pybtex “keystyle” plugin used by both Sphinx projects. |
| catalog/VERSION | Add initial catalog version marker. |
| catalog/test_build.py | Update tests for catalog/build.py behavior, new paths, and manifest/citation features. |
| catalog/hierarchy.yaml | Add human-readable labels for catalog hierarchy nodes. |
| catalog/gtdraw_settings.yaml | Add default and per-slug gtdraw settings for catalog rendering. |
| catalog/games/manifest.json | Add committed GUI-facing manifest (title/desc/grouping/stats per game). |
| catalog/games/img/.gitkeep | Ensure image output directory exists in-tree. |
| catalog/games/books/myerson1991/fig2_1.efg | Add catalog game file. |
| catalog/games/books/myerson1991/fig4_2.efg | Add catalog game file. |
| catalog/games/books/shohamleytonbrown2008/fig5_1.efg | Add catalog game file. |
| catalog/games/books/shohamleytonbrown2008/fig5_1__original_layout.ef | Add curated layout variant. |
| catalog/games/books/shohamleytonbrown2008/fig5_2.efg | Add catalog game file. |
| catalog/games/books/shohamleytonbrown2008/fig5_2__original_layout.ef | Add curated layout variant. |
| catalog/games/books/shohamleytonbrown2008/fig5_9.efg | Add catalog game file. |
| catalog/games/books/shohamleytonbrown2008/fig5_9__original_layout.ef | Add curated layout variant. |
| catalog/games/books/shohamleytonbrown2008/fig5_10.efg | Add catalog game file. |
| catalog/games/books/shohamleytonbrown2008/fig5_10__original_layout.ef | Add curated layout variant. |
| catalog/games/books/shohamleytonbrown2008/fig5_11.efg | Add catalog game file. |
| catalog/games/books/shohamleytonbrown2008/fig5_11__original_layout.ef | Add curated layout variant. |
| catalog/games/books/shohamleytonbrown2008/fig5_12.efg | Add catalog game file. |
| catalog/games/books/shohamleytonbrown2008/fig5_12__original_layout.ef | Add curated layout variant. |
| catalog/games/books/shohamleytonbrown2008/fig5_15.efg | Add catalog game file. |
| catalog/games/books/shohamleytonbrown2008/fig5_15__original_layout.ef | Add curated layout variant. |
| catalog/games/books/shohamleytonbrown2008/fig6_2.efg | Add catalog game file. |
| catalog/games/books/shohamleytonbrown2008/fig6_2__original_layout.ef | Add curated layout variant. |
| catalog/games/books/shohamleytonbrown2008/fig6_8.efg | Add catalog game file. |
| catalog/games/books/shohamleytonbrown2008/fig6_8__original_layout.ef | Add curated layout variant. |
| catalog/games/books/vonstengel2022/fig10.1.efg | Add catalog game file. |
| catalog/games/books/vonstengel2022/fig10.12.efg | Add catalog game file. |
| catalog/games/books/vonstengel2022/fig10.5.efg | Add catalog game file. |
| catalog/games/books/vonstengel2022/fig10.7.efg | Add catalog game file. |
| catalog/games/books/watson2013/exercise29_6.efg | Add catalog game file. |
| catalog/games/books/watson2013/fig29_1.efg | Add catalog game file. |
| catalog/games/conf/itcs/jakobsen2016/fig1a.efg | Add catalog game file. |
| catalog/games/conf/itcs/jakobsen2016/fig1b.efg | Add catalog game file. |
| catalog/games/conf/itcs/jakobsen2016/fig1c.efg | Add catalog game file. |
| catalog/games/conf/itcs/jakobsen2016/fig3.efg | Add catalog game file. |
| catalog/games/journals/dcg/vonstengel1999/6x6_game_with_75_eq.nfg | Add/update catalog game file. |
| catalog/games/journals/dcg/vonstengel1999/6x6_game_with_75_eq_small_payoffs.nfg | Add catalog game file. |
| catalog/games/journals/geb/bagwell1995.efg | Add catalog game file. |
| catalog/games/journals/geb/gilboa1997/fig1.efg | Add catalog game file. |
| catalog/games/journals/geb/gilboa1997/fig2.efg | Add catalog game file. |
| catalog/games/journals/geb/wichardt2008.efg | Add catalog game file. |
| catalog/games/journals/ijgt/nau2004/sec3.nfg | Add catalog game file. |
| catalog/games/journals/ijgt/nau2004/sec4.nfg | Add catalog game file. |
| catalog/games/journals/ijgt/nau2004/sec5.nfg | Add catalog game file. |
| catalog/games/journals/ijgt/nau2004/sec6.nfg | Add catalog game file. |
| catalog/games/journals/ijgt/selten1975/fig1.efg | Add catalog game file. |
| catalog/games/journals/ijgt/selten1975/fig2.efg | Add catalog game file. |
| catalog/games/journals/ijgt/selten1975/fig3.efg | Add catalog game file. |
| catalog/games/journals/mor/vonstengelforges2008/fig1.efg | Add catalog game file. |
| catalog/games/journals/mor/vonstengelforges2008/fig1__Original_Layout.ef | Add curated layout variant. |
| catalog/games/journals/mor/vonstengelforges2008/fig6.efg | Add catalog game file. |
| catalog/games/journals/mor/vonstengelforges2008/fig6__Original_Layout.ef | Add curated layout variant. |
| catalog/games/journals/mor/vonstengelforges2008/fig9.efg | Add catalog game file. |
| catalog/games/journals/mor/vonstengelforges2008/fig9__Original_Layout.ef | Add curated layout variant. |
| catalog/games/journals/other/reiley2008/fig1.efg | Add catalog game file. |
| catalog/games/journals/other/shapley1974/fig2.nfg | Add catalog game file. |
| catalog/games/journals/other/shapley1974/fig3.nfg | Add catalog game file. |
| catalog/doc/index.rst | Add separate catalog Sphinx project entry page. |
| catalog/doc/developer.rst | Add catalog-specific developer workflow docs for contributing games. |
| catalog/doc/conf.py | Add separate Sphinx config for catalog docs (shared bib style import). |
| catalog/catalog.am | Add new catalog file list for build/distribution. |
| catalog/build.py | Replace legacy update script with new build script (table + manifest + catalog.am). |
| catalog/.readthedocs.yml | Add separate ReadTheDocs configuration for catalog docs build. |
| build_support/msw/gambit.wxs.in | Add catalog folder and component group reference for MSI packaging. |
| build_support/catalog/catalog.am | Remove legacy catalog make fragment (replaced by catalog/catalog.am). |
| .readthedocs.yml | Stop generating catalog table in main RTD build; clarify gtdraw usage note. |
| .gitignore | Update ignores for new catalog doc/table and generated image locations. |
| .github/workflows/tools.yml | Include catalog files in Windows packaging job (heat/candle/light). |
| .github/workflows/lint.yml | Add CI job to enforce catalog/catalog.am and catalog/games/manifest.json sync. |
Review details
Suppressed comments (3)
catalog/build.py:480
- This docstring claims build_manifest() “Does not require gtdraw”, but the module currently imports gtdraw at import time. Either defer the gtdraw import (so build_manifest can truly be used without it) or adjust this docstring to avoid documenting behavior the module doesn’t currently support.
src/gui/catalogdata.cc:439 - This parse-failure message is user-visible but isn’t marked for translation, and e.what() should be converted to wxString explicitly to avoid encoding surprises.
src/gui/catalogdata.cc:446 - This malformed-manifest message is user-visible but isn’t marked for translation. Wrapping it in _() keeps catalog-browser error text consistent with the rest of the GUI.
- Files reviewed: 48/104 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+359
to
+362
| JsonValue result; | ||
| result.m_type = JsonValue::Type::Number; | ||
| result.m_numberValue = std::stod(m_text.substr(start, m_pos - start)); | ||
| return result; |
Comment on lines
+421
to
+425
| std::ifstream file(manifestPath.ToStdString(), std::ios::binary); | ||
| if (!file) { | ||
| if (p_error) { | ||
| *p_error = wxString::Format("Could not open catalog manifest at %s", manifestPath); | ||
| } |
rahulsavani
reviewed
Sep 10, 2026
rahulsavani
left a comment
rahulsavani
left a comment
Member
There was a problem hiding this comment.
This works well for me, great stuff.
One idea: We might want to allow sorting by column in the catalog table, which would e.g., give an easy way, without a search function, to see all 3 player games together, or find a particular entry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.