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

[Fix #357] Update AreaEditor8a.cs to save AreaSettings #358

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

drvictorvs
Copy link
Contributor

@drvictorvs drvictorvs commented Jan 31, 2024

So, I think this did it. Well, I'm sure it works, the Field/Areas Settings tab will save as intended, I'm just not sure whether it creates other problems. So I did it in the most analogous way to SaveArea, without any code cleaning, in the wrong place, just so that it's very explicit what it's doing. So feel free to edit it.

By the way, I managed to find that Flag03 is essentially "Can Ride", i.e., if you enable it in Jubilife Village, you'll be able to ride Pokémon there.

Best regards.

Copy link
Collaborator

@duckdoom4 duckdoom4 left a comment

Choose a reason for hiding this comment

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

Looks good to me! Tho I do wonder if we need the additional function for writing the object. Can't we use one of the already available methods?

I'll approve it tho, I don't see any issues with it. Maybe @kwsch has a comment?

@drvictorvs
Copy link
Contributor Author

drvictorvs commented Feb 1, 2024

Looks good to me! Tho I do wonder if we need the additional function for writing the object. Can't we use one of the already available methods?

I'll approve it tho, I don't see any issues with it. Maybe @kwsch has a comment?

Thank you! My first idea was to simply use TryWrite(string, flatbuffer) to write Settings to bin/field/resident/AreaSettings.bin, but the Settings in ResidentArea.cs is not the same as in AreaEditor8a.cs. LoadAreaByName() sends just the info for the active area (using the AreaSettingsTable.Find method), and TryWrite() is not set up to deal with that.

There are ways to get around that but I don't understand nearly enough of C# to understand the downsides to each of them. (I literally started learning it just to solve this problem.) So I decided to defer to someone else's knowledge.

If I had to choose, I'd add an argument to the ResidentArea class:

public sealed class ResidentArea(GFPack resident, AreaSettings settings, >> AreaSettingsTable? table = null <<)
{
public AreaSettingsTable? Table = table;
...

Modify LoadAreaByName() in AreaEditor8a.cs (and revert the other changes):

var area = new ResidentArea(Resident, Settings.Find(name), >> Settings <<);

(Ideally I suppose you'd pass Settings and name and let ResidentArea.cs figure out the rest, but that would break AreaInstance.cs and probably other things.)

Then add to SaveInfo() (in ResidentArea.cs):

        if(Table != null){
            TryWrite("bin/field/resident/AreaSettings.bin", Table);
        }

I have tested this and it seems to work (I haven't tested the resulting file in-game yet).

@duckdoom4
Copy link
Collaborator

duckdoom4 commented Feb 1, 2024

Ah sorry for the confusion. What I mean is that you can use FlatBufferConverter.SerializeFrom(). (I just checked, and that is the same code as the Write function you added).

Edit: Also

I managed to find that Flag03 is essentially "Can Ride"

That's a great find, if you want you can rename it in the fbs file and where it's used so we can document it :D

@duckdoom4
Copy link
Collaborator

@drvictorvs If you can change that thing I mentioned I'll merge it in :)

return;

byte[] result = Write(obj);
Resident[index] = result;
Copy link
Collaborator

Choose a reason for hiding this comment

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

So this can be FlatBufferConverter.SerializeFrom(obj)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants