StevensSEC / pokemonbattlelib

A portable library for accurately simulating Pokemon battles.
GNU General Public License v3.0
12 stars 2 forks source link

fix: JSON encoding for battle context #376

Closed adapap closed 3 years ago

adapap commented 3 years ago

Didn't test this.

Not sure how to approach the other one, do we need a MarshalJSON for AgentTarget as well?

fixes #374 fixes #373

codecov[bot] commented 3 years ago

Codecov Report

Merging #376 (0c46a7a) into main (b2fced1) will increase coverage by 0.6352%. The diff coverage is 93.1818%.

:exclamation: Current head 0c46a7a differs from pull request most recent head fc8f9e7. Consider uploading reports for the commit fc8f9e7 to get more accurate results Impacted file tree graph

@@               Coverage Diff                @@
##               main       #376        +/-   ##
================================================
+ Coverage   94.5475%   95.1827%   +0.6352%     
================================================
  Files            16         16                
  Lines          1779       1806        +27     
================================================
+ Hits           1682       1719        +37     
+ Misses           77         66        -11     
- Partials         20         21         +1     
Impacted Files Coverage Δ
battle_round.go 94.0476% <ø> (-0.3658%) :arrow_down:
move.go 90.0000% <ø> (ø)
pokemon.go 90.3409% <ø> (+4.5454%) :arrow_up:
transactions.go 96.1065% <50.0000%> (+0.0079%) :arrow_up:
battle.go 96.1783% <95.2381%> (+2.8450%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b2fced1...fc8f9e7. Read the comment docs.

dyc3 commented 3 years ago

Yeah this is stumping me too. AgentTarget does need to contain party and slot in order to unmarshal it back out into a usable form. I was able to add it to the marshalling, but it still doesn't unmarshal.

adapap commented 3 years ago

@dyc3 I think this might be the issue but I am not exactly sure how to fix it?

func HandleDumbAgent(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    // below is hard-coded response that does not use AgentTarget
    w.Write([]byte(`{"type": 0, "args": {"target": {"party":0, "slot": 0}, "move": 0}}`))
}
dyc3 commented 3 years ago

That endpoint is no longer used and should probably be removed. It also doesn't get used in the tests that are failing.

adapap commented 3 years ago

I'm looking at this again - never even realized tests were failing. Do we even want to test marshal/unmarshal of target? I think Go handles marshalling of structs pretty well by default, and I think making sure AgentTarget is handled with JSON is more important.

Edit: added a potential fix, seems like the "issue" is revolving around the fact that unexported struct fields do not get marshalled/unmarshalled with Go's standard JSON marshalling.

dyc3 commented 3 years ago

Do we even want to test marshal/unmarshal of target?

Given that the tests are failing, I think that's a pretty clear indicator that it needs to be tested for. Ideally, we could use optic to test the http api end-to-end, but that's a no-go currently. Optic doesn't record query parameters and that is what the http api is based on. I'm starting to wish I wrote tests for the http api...

dyc3 commented 3 years ago

OK, I don't think I explained the problem well enough, my bad.

In order for an agent to make an informed decision for a turn, they need the info in the battle context.

Here's how to create a battle: POST http://localhost:4000/battle/new with the body:

{
    "teams": [
        {
            "parties": [
                {
                    "pokemon": [
                        {
                            "Name": "Pichu", "NatDex": 172, "Level": 79, "Ability": 98, "TotalExperience": 493039, "Gender": 1, "IVs": [1, 2, 4, 0, 2, 1], "EVs": [18, 17, 22, 32, 21, 41], "Nature": 8, "Stats": [124, 72, 35, 66, 65, 108], "StatModifiers": [0, 0, 0, 0, 0, 0, 0, 0, 0], "StatusEffects": 0, "CurrentHP": 124, "Moves": [{"Name": "Trick Room", "Type": 8192, "Category": 0, "Targets": 12, "Priority": -7, "Power": 0, "Accuracy": 0, "InitialMaxPP": 0, "Id": 433, "CurrentPP": 0, "MaxPP": 0}, {"Name": "Dragon Breath", "Type": 32768, "Category": 2, "Targets": 10, "Priority": 0, "Power": 60, "Accuracy": 100, "InitialMaxPP": 0, "Id": 225, "CurrentPP": 0, "MaxPP": 0}, {"Name": "Metal Claw", "Type": 256, "Category": 1, "Targets": 10, "Priority": 0, "Power": 50, "Accuracy": 95, "InitialMaxPP": 0, "Id": 232, "CurrentPP": 0, "MaxPP": 0}, {"Name": "Blizzard", "Type": 16384, "Category": 2, "Targets": 11, "Priority": 0, "Power": 110, "Accuracy": 70, "InitialMaxPP": 0, "Id": 59, "CurrentPP": 0, "MaxPP": 0}], "Friendship": 0, "OriginalTrainerID": 0, "Type": 4096
                        }
                    ]
                }
            ]
        }, 
        {
            "parties": [
                {
                    "pokemon": [
                        {
                            "NatDex": 151, "Level": 6, "Ability": 108, "TotalExperience": 179, "Gender": 0, "IVs": [3, 1, 1, 4, 0, 3], "EVs": [7, 24, 41, 11, 3, 20], "Nature": 8, "Stats": [28, 17, 17, 17, 17, 17], "StatModifiers": [0, 0, 0, 0, 0, 0, 0, 0, 0], "StatusEffects": 0, "CurrentHP": 28, "Moves": [{"Name": "Bubble Beam", "Type": 1024, "Category": 2, "Targets": 10, "Priority": 0, "Power": 65, "Accuracy": 100, "InitialMaxPP": 0, "Id": 61, "CurrentPP": 0, "MaxPP": 0}, {"Name": "Head Smash", "Type": 32, "Category": 1, "Targets": 10, "Priority": 0, "Power": 150, "Accuracy": 80, "InitialMaxPP": 0, "Id": 457, "CurrentPP": 0, "MaxPP": 0}, {"Name": "Draco Meteor", "Type": 32768, "Category": 2, "Targets": 10, "Priority": 0, "Power": 130, "Accuracy": 90, "InitialMaxPP": 0, "Id": 434, "CurrentPP": 0, "MaxPP": 0}, {"Name": "Hammer Arm", "Type": 2, "Category": 1, "Targets": 10, "Priority": 0, "Power": 100, "Accuracy": 90, "InitialMaxPP": 0, "Id": 359, "CurrentPP": 0, "MaxPP": 0}], "Friendship": 0, "OriginalTrainerID": 0, "Type": 8192
                        }
                    ]
                }
            ]
        }
    ]
}

After you create a battle, you can get the battle context.

This is what currently gets returned for /battle/context?id=0&target=0

{
    "Self": {
        "Party": 0,
        "Slot": 0
    },
    "Allies": [
        {
            "Party": 0,
            "Slot": 0
        }
    ],
    "Opponents": [
        {
            "Party": 1,
            "Slot": 0
        }
    ],
    "Targets": [
        {
            "Party": 0,
            "Slot": 0
        },
        {
            "Party": 1,
            "Slot": 0
        }
    ],
    "Battle": {
        "Weather": 0,
        "ShiftSet": false,
        "State": 1
    },
    "Team": 0
}

This contains the targets, but they do not contain any information about the pokemon, just the party and slot. It needs to contain information about the pokemon, because there is no other way to get information about the battlefield via the http api.

In other words, AgentTarget needs to contain the pokemon when marshalled, and it can ignore that field when unmarshalling.

adapap commented 3 years ago

I'm unassigning myself from this - too difficult for me at the moment

dyc3 commented 3 years ago

I was able to get it to work. I had to get rid of the type alias thing and just put in the fields manually.