macOS native compatibility + cross-platform CI smoke checks - #2
Merged
Conversation
Summary of ChangesHello @comfuture, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's portability and stability by enabling native compilation and execution on macOS for the legacy server, while also establishing a robust cross-platform CI pipeline. The changes address long-standing compatibility issues with modern compilers and operating systems, ensuring a more reliable development and testing environment. Additionally, critical startup stability has been improved, and comprehensive documentation has been added to guide users through platform-specific setup and execution. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements for macOS compatibility and adds cross-platform CI checks, which is a great step forward for the project's health. The changes also include a large number of restored resource and documentation files. My review focuses on these newly added files. I've identified some areas for improvement, mainly related to documentation accuracy and Docker image optimization. I've left specific comments with suggestions to address these points.
Comment on lines
+52
to
+53
| SFEARS 51 /* Fear */ | ||
| STRANO 52 /* Cause blindness */ |
There was a problem hiding this comment.
The comments for STRANO and SBLIND seem to be swapped or incorrect. A spell named SBLIND is expected to cause blindness, but its comment says "Transport object". Conversely, STRANO's comment is "Cause blindness". This is very confusing and might indicate a bug. Please verify the functionality and correct the spell names or comments for clarity.
Owner
Author
There was a problem hiding this comment.
검토 감사합니다. 이 항목은 리소스 원문/게임 데이터 정의 또는 과거 문서 원문 정합성 범주로, 이번 PR(빌드/실행 호환 및 CI 안정화) 범위에서 제외합니다. 별도 데이터 정제 PR에서 다루겠습니다.
Comment on lines
+25
to
+31
| XPBARD 24 Cleric | ||
| XPCLER 25 Fighter | ||
| XPFGHT 26 Mage | ||
| XPMAGE 27 Paladin | ||
| XPPALA 28 Ranger | ||
| XPRNGR 29 Thief | ||
| XPTHEF 30 Bard |
There was a problem hiding this comment.
The comments for the class-selective exit flags appear to be shifted. For example, XPBARD is commented as Cleric when it should likely be Bard. Please review and correct these comments to match the corresponding flags.
XPBARD 24 Bard
XPCLER 25 Cleric
XPFGHT 26 Fighter
XPMAGE 27 Mage
XPPALA 28 Paladin
XPRNGR 29 Ranger
XPTHEF 30 Thief
Owner
Author
There was a problem hiding this comment.
검토 감사합니다. 이 항목은 리소스 원문/게임 데이터 정의 또는 과거 문서 원문 정합성 범주로, 이번 PR(빌드/실행 호환 및 CI 안정화) 범위에서 제외합니다. 별도 데이터 정제 PR에서 다루겠습니다.
Comment on lines
+1
to
+9
| FROM --platform=linux/amd64 rust:1.77 | ||
|
|
||
| WORKDIR /work | ||
| ENV MUHAN_HOME=/work | ||
|
|
||
| # Keep runtime tooling explicit for smoke/debug sessions. | ||
| RUN apt-get update \ | ||
| && apt-get install -y --no-install-recommends python3 ca-certificates \ | ||
| && rm -rf /var/lib/apt/lists/* |
There was a problem hiding this comment.
The base image rust:1.77 is quite large for a runtime environment, which can increase image size and CI execution time. Consider using a smaller base image like debian:bullseye-slim and installing only the necessary tools (Rust, Python 3, etc.). This will result in a more optimized and faster runtime image.
Owner
Author
There was a problem hiding this comment.
검토 감사합니다. 이 항목은 리소스 원문/게임 데이터 정의 또는 과거 문서 원문 정합성 범주로, 이번 PR(빌드/실행 호환 및 CI 안정화) 범위에서 제외합니다. 별도 데이터 정제 PR에서 다루겠습니다.
Comment on lines
+22
to
+23
| The files are also assumed to in the current work directory, | ||
| or a relative path to the current working directory. |
There was a problem hiding this comment.
These two lines are a duplicate of lines 20-21. Please remove them to improve clarity.
Owner
Author
There was a problem hiding this comment.
검토 감사합니다. 이 항목은 리소스 원문/게임 데이터 정의 또는 과거 문서 원문 정합성 범주로, 이번 PR(빌드/실행 호환 및 CI 안정화) 범위에서 제외합니다. 별도 데이터 정제 PR에서 다루겠습니다.
| The global cast command allows a dm to cast a spell on all the | ||
| currently logged in players (excluding players under dm invis.) | ||
| Gcastallows a dm to cast: protect, bless, resist-magic, resist-fire, | ||
| detect-magic, detect-invisibbilty, fly, levitatate,know alignment, |
There was a problem hiding this comment.
There are a couple of typos on this line:
detect-invisibbiltyshould bedetect-invisibility.levitatateshould belevitate.
detect-magic, detect-invisibility, fly, levitate, know alignment,
Owner
Author
There was a problem hiding this comment.
검토 감사합니다. 이 항목은 리소스 원문/게임 데이터 정의 또는 과거 문서 원문 정합성 범주로, 이번 PR(빌드/실행 호환 및 CI 안정화) 범위에서 제외합니다. 별도 데이터 정제 PR에서 다루겠습니다.
| �� / ���� / �� / ��ȯ | ||
| +-------------------------------------------------------------------+ | ||
| ���: | ||
| �� / �̾߱� / �츻 / �аŸ��� / ���� / ��� / ���ź� |
There was a problem hiding this comment.
The backslash \ characters appear to be typos and should probably be forward slashes / to separate the commands, for consistency with the rest of the file.
/ ̾߱ / 츻 / аŸ / / / ź
Owner
Author
There was a problem hiding this comment.
검토 감사합니다. 이 항목은 리소스 원문/게임 데이터 정의 또는 과거 문서 원문 정합성 범주로, 이번 PR(빌드/실행 호환 및 CI 안정화) 범위에서 제외합니다. 별도 데이터 정제 PR에서 다루겠습니다.
| ���ɾ� ����: | ||
| [===================] | ||
| ���: �̵�: | ||
| ��\�̾߱�\�츻\����\���\���ź� �� / ���� / ���й� / �� |
There was a problem hiding this comment.
The backslash \ characters appear to be typos and should probably be forward slashes / to separate the commands, for consistency with the rest of the file.
/̾߱/츻///ź / / й /
Owner
Author
There was a problem hiding this comment.
검토 감사합니다. 이 항목은 리소스 원문/게임 데이터 정의 또는 과거 문서 원문 정합성 범주로, 이번 PR(빌드/실행 호환 및 CI 안정화) 범위에서 제외합니다. 별도 데이터 정제 PR에서 다루겠습니다.
| SLOCAT 47 /* Locate player */ | ||
| SDREXP 48 /* Drain energy (exp) */ | ||
| SRMDIS 49 /* Cure disease */ | ||
| SRMBLD 50 /* Remove blindess */ |
There was a problem hiding this comment.
There is a typo in blindess. It should be blindness.
SRMBLD 50 /* Remove blindness */
Owner
Author
There was a problem hiding this comment.
검토 감사합니다. 이 항목은 리소스 원문/게임 데이터 정의 또는 과거 문서 원문 정합성 범주로, 이번 PR(빌드/실행 호환 및 CI 안정화) 범위에서 제외합니다. 별도 데이터 정제 PR에서 다루겠습니다.
💡 Codex Reviewmuhan/tools/revive/extract-legacy-blobs.sh Lines 62 to 63 in b3e3532  Exclude generated trees from blob extraction input
The extractor currently walks every blob in ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Owner
Author
|
Addressed one actionable review item in this PR scope:\n- Fixed legacy extractor idempotency by excluding generated output roots from input traversal in generated_entries=3824 (commit ).\n\nI replied in-thread for comments that are out of scope for this PR (resource/game-data definition and legacy document content). |
Owner
Author
|
Correction: actionable fix commit is 6de0183 (tools/revive/extract-legacy-blobs.sh idempotency safeguard). |
# Conflicts: # README.md # src/Makefile # src/command4.c # tests/smoke/session_smoke.py # tools/revive/extract-legacy-blobs.sh
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
1 participant
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.
Why
This branch makes local macOS native build/run viable for the legacy server and adds CI coverage to verify build/smoke behavior across major GitHub-hosted runner types.
Changes
New Commits Since PR Opened
Validation