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

chore: rename engine-module.txt to module.txt #4722

Merged
merged 6 commits into from
May 30, 2021

Conversation

jdrueckert
Copy link
Member

@jdrueckert jdrueckert commented May 28, 2021

Resolves #4646

Tbh I have no idea, if this is what's expected to be done to resolve #4646 ...
I just thought I'd go for what I understood and use that as a basis for discussion πŸ€·β€β™€οΈ πŸ˜…

@jdrueckert jdrueckert added Status: Needs Discussion Requires help discussing a reported issue or provided PR Type: Chore Request for or implementation of maintenance changes labels May 28, 2021
@jdrueckert jdrueckert requested a review from keturn May 28, 2021 18:31
@jdrueckert jdrueckert self-assigned this May 28, 2021
Copy link
Member

@keturn keturn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the one thing you left out of this PR titled "rename engine-module.txt to module.txt" was renaming engine-module.txt. 😜

@jdrueckert
Copy link
Member Author

jdrueckert commented May 29, 2021

It looks like the one thing you left out of this PR titled "rename engine-module.txt to module.txt" was renaming engine-module.txt. stuck_out_tongue_winking_eye

Whoops πŸ˜… Should be done now

@jdrueckert jdrueckert requested a review from keturn May 29, 2021 22:29
@jdrueckert jdrueckert removed the Status: Needs Discussion Requires help discussing a reported issue or provided PR label May 29, 2021
@keturn
Copy link
Member

keturn commented May 29, 2021

Ah, now it runs with passing tests!

I see versionInfo.properties is built with version information, and I believe the test cases will make sure that ModuleManager has a module of the appropriate name.

I think that the only thing left to confirm is that you can actually run a game world in a real process with other (non-engine) modules present. Can't think of anything else in particular to look for beyond that.

@keturn keturn added the Status: Needs Testing Requires to be tested in-game for reproducibility label May 30, 2021
@pollend
Copy link
Member

pollend commented May 30, 2021

@jdrueckert do you want to merge this?

@jdrueckert
Copy link
Member Author

@jdrueckert do you want to merge this?

need to test it properly (as keturn mentioned) first, but once that's done: yes.
any objections? @pollend

@jdrueckert
Copy link
Member Author

tests out fine in CoreGameplay, so I'll go ahead and merge this

@jdrueckert jdrueckert merged commit 7958b66 into develop May 30, 2021
@jdrueckert jdrueckert deleted the chore/rename-engine-module-txt branch May 30, 2021 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Testing Requires to be tested in-game for reproducibility Type: Chore Request for or implementation of maintenance changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't use engine-module.txt as a possible metadata source for all modules
3 participants