WIP: [AI] Adding Housing Support #122
No reviewers
Labels
No labels
bug
dependencies
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
IllusionMods/BepisPlugins!122
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "hooh-hooah/master"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This is a Draft Pull Request. requires constant review of code until all checklists are checked
Adding Housing support for AI
Since AI's main game requires some nice touch when it comes to the main game, I decided to do some action to add some new features for the game.
But since this is the first time to be a contributor for the bepis plugins, I've made this "WIP" pull request.
Origin Branch
hooh-hooah:masterhttps://github.com/hooh-hooah/BepisPlugins
Related Plugin
https://github.com/hooh-hooah/IL_MapIntegrationPlugin
Checklists
Loading housing information from the CSV file in the zipmod files.
Adding Extended Save for the main-game files
Housing Item Sideloader ID Resolver
Testing
Downloading AI_MapIntegrationPlugin and the AI POC mod
https://mega.nz/folder/po0QRaSZ#xrXPDDynGtS81LqCgyIXYA
Leave a comment when it looks like going wrong.
@hooh-hooah Looks good so far, but needs some more work before it's fully usable like you've mentioned. Thanks for working on this!
Ye it's not going to be done in a short time but I expect it's going to be done in a month I guess. Summarizing the concepts and milestones before adding id resolve on furniture.
Aww heck
The housing data and the main-game save data are glued together!
Well finally, analysis of the AI Shoujyo's Save File is done.
It's bit weird but i'll try to post the structure of file
@ManlyMarco I have a question at this moment, Since my changes are solely for AI's main-game, am I going right direction with keeping asome resolvers and listloaders inside of "AI_Sideloader" instead of "Core_Sideloader"?
I'm trying to match the style with rest of the code but this is the one of few things that makes me confused.
Good work on the PR so far, thank you!
I noticed some things that could be improved, it would be great if you could give it a look. If you have any questions about my comments, you can ping me on discord whenever.
To add to the comments:
Since the main game and housing ext data code is for the most part stand-alone, it would be nice to have it be separated into separate .cs files, like I did with the main game save data in KK here
IllusionMods/BepisPlugins@c5916e6797/src/KK_ExtensibleSaveFormatIf you want, I can ask others to review the changes as well, so we can find issues and get it merged faster.
@ -0,0 +1,103 @@using System;Maybe rename these events to follow the existing main game save data events for KK?
IllusionMods/BepisPlugins@c5916e6797/src/KK_ExtensibleSaveFormat/KK.ExtendedSave.SaveData.cs (L55-L60)@ -11,2 +19,4 @@const string MainGameSaveMarker = "WD";//Override ExtSave for list loading at game startup[HarmonyPrefix]Why LogMessage? I assume it's leftover debug code.
Missing docs, either fill in or remove completely. There's no need to add docs in internal and private classes unless it's useful for other developers.
It's better to use a constant for the marker to avoid mistakes in the future, also a longer marker like
extWDwould be saferDoes it work on the Steam version, and Japanese versions both with and without DX?
@ -22,0 +60,4 @@try{string marker = br.ReadString();int version = br.ReadInt32();Either use AccessTools or throw an exception on null when getting types like this, because if the type for some reason can't be found then it's impossible to know which type is missing without further debugging since all you get is a null reference exception. (AccessTools will log a warning to the log if a type can't be found, or you can do
?? throw new ArgumentException("Type.Method couldn't be found"))@ -0,0 +9,4 @@{internal static partial class Hooks{internal static void InstallMainGameHooks(Harmony harmony)Unnecessary method or forgot to patch?
@ -0,0 +8,4 @@{internal static partial class Lists{// internal main game furniture dataMove above to be a comment of class Lists or it can get lost if someone does any refactoring
@ -0,0 +13,4 @@/// <summary>/// Currently only resolving/// </summary>/// <param name="stream"></param>Remove any empty parts of docs when the PR is ready for merging so there's a warning if it does need to be filled in
@ -13,6 +13,7 @@ using System.IO;using System.Linq;It's best to avoid unnecessary changes since it's harder to see what parts actually matter and it messes with git history
Missing code inside the #if ?
@ -299,1 +311,4 @@#endif#endif#if AI// AI Main-Game Loading ProcedureThis will only work in Japanese version of the game since the Steam version has a different name. Compare to the studio name instead.
Broken #if ?
Yes, since it's only useful for AI it's better to keep it in the AI project. Like I mentioned in the massive dump above, you can separate this functionality into a bunch of new files so it's easy to refactor in the future if necessary.
@ -0,0 +1,103 @@using System;Oh man I didn't noticed those one. I'll follow the name.
@ -11,2 +19,4 @@const string MainGameSaveMarker = "WD";//Override ExtSave for list loading at game startup[HarmonyPrefix]Yeah, I've tested almost everything is working as intended i'll clean up the rest of the code to make it fit more nicer.
@ -11,2 +19,4 @@const string MainGameSaveMarker = "WD";//Override ExtSave for list loading at game startup[HarmonyPrefix]noted.
@ -11,2 +19,4 @@const string MainGameSaveMarker = "WD";//Override ExtSave for list loading at game startup[HarmonyPrefix]Well "WD" was totally random token in my mind when i wrote this one. I'll try to make it as constant when there is no other better way to separate world data
@ -13,6 +13,7 @@ using System.IO;using System.Linq;Oof okay
Probably left over while moving code from the common codebase. As I stated, the code cleanup will be done soon since the most of features are working.
@ -299,1 +311,4 @@#endif#endif#if AI// AI Main-Game Loading ProcedureThanks for the tip. I'll keep that in my mind.
https://github.com/IllusionMods/BepisPlugins/pull/122#discussion_r576165332
@ -11,2 +19,4 @@const string MainGameSaveMarker = "WD";//Override ExtSave for list loading at game startup[HarmonyPrefix]Well currently I don't have multiple versions of AI atm. I need to figure out how to obtain old versions first
Thanks for the review, that was really helpful. I'll commit the fix when it's ready.
@ -13,6 +13,7 @@ using System.IO;using System.Linq;https://github.com/IllusionMods/BepisPlugins/pull/122/commits/a57e34b7c5f0c984a134ea1e0023d8c6bfee7d86
https://github.com/IllusionMods/BepisPlugins/pull/122/commits/a57e34b7c5f0c984a134ea1e0023d8c6bfee7d86
@ -299,1 +311,4 @@#endif#endif#if AI// AI Main-Game Loading Procedurehttps://github.com/IllusionMods/BepisPlugins/pull/122/commits/a57e34b7c5f0c984a134ea1e0023d8c6bfee7d86
https://github.com/IllusionMods/BepisPlugins/pull/122/commits/a57e34b7c5f0c984a134ea1e0023d8c6bfee7d86
@ -11,2 +19,4 @@const string MainGameSaveMarker = "WD";//Override ExtSave for list loading at game startup[HarmonyPrefix]https://github.com/IllusionMods/BepisPlugins/pull/122/commits/a57e34b7c5f0c984a134ea1e0023d8c6bfee7d86
@ -11,2 +19,4 @@const string MainGameSaveMarker = "WD";//Override ExtSave for list loading at game startup[HarmonyPrefix]https://github.com/IllusionMods/BepisPlugins/pull/122/commits/a57e34b7c5f0c984a134ea1e0023d8c6bfee7d86
@ -11,2 +19,4 @@const string MainGameSaveMarker = "WD";//Override ExtSave for list loading at game startup[HarmonyPrefix]https://github.com/IllusionMods/BepisPlugins/pull/122/commits/a57e34b7c5f0c984a134ea1e0023d8c6bfee7d86
@ -0,0 +1,103 @@using System;https://github.com/IllusionMods/BepisPlugins/pull/122/commits/a57e34b7c5f0c984a134ea1e0023d8c6bfee7d86
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.