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 RawMemory #7

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

Conversation

RiftLurker
Copy link
Contributor

@RiftLurker RiftLurker commented Jan 2, 2017

This allows actually using the custom serialization explained at
http://support.screeps.com/hc/en-us/articles/203016642-Working-with-memory

Right now Creep#memory, Flag#memory, Room#memory and Spawn#Memory always
access globals.Memory which is not changeable from inside users code.
Therefor Memory parsing will happen twice, once by the users code and
once by the first access to globals.Memory, resulting in two different
objects.

The solution to this would be tracking down why setting Memory (or
global.Memory) in code doesn't change globals.Memory, but I don't
have enough experience with the screeps server and the vm module.

For now this just makes the memory property configurable, so we can
change it to actually access our version of the parsed Memory.

Test case:

module.exports.loop = function loop() {
    Memory = JSON.parse(RawMemory.get());

    const creepName = "test";
    const creep = Game.creeps[creepName];

    if (!creep) {
        _.first(_.values(Game.spawns)).createCreep([MOVE], creepName);
    } else {
        console.log(`--- Test case (tick: ${Game.time}) ---`);
        Memory.creeps[creepName].tick = Game.time;

        const gMemory = Memory.creeps[creepName].tick;
        const cMemory = creep.memory.tick;

        if (gMemory === cMemory) {
            console.log(`success: ${gMemory} === ${cMemory}`);
        } else {
            console.log(`failed: ${gMemory} !== ${cMemory}`);
        }
    }

    RawMemory.set(JSON.stringify(Memory));
}

@RiftLurker
Copy link
Contributor Author

RiftLurker commented Jan 2, 2017

Update: with the latest commit adding a setter to the Memory property of runCodeCache[userId].globals it is now possible to use custom serialization exactly like the article Working with memory explains.

This allows actually using the custom serialization explained at
http://support.screeps.com/hc/en-us/articles/203016642-Working-with-memory

Right now `Creep#memory`, `Flag#memory`, `Room#memory` and `Spawn#Memory` always
access `globals.Memory` which is not changeable from inside users code.
Therefor Memory parsing will happen twice, once by the users code and
once by the first access to `globals.Memory`, resulting in two different
objects.

The solution to this would be tracking down why setting `Memory` (or
`global.Memory`) in code doesn't change `globals.Memory`, but I don't
have enough experience with the screeps server and the vm module.

For now this just makes the `memory` property configurable, so we can
change it to actually access our version of the parsed Memory.

Test case:
```
module.exports.loop = function loop() {
    Memory = JSON.parse(RawMemory.get());

    const creepName = "test";
    const creep = Game.creeps[creepName];

    if (!creep) {
        _.first(_.values(Game.spawns)).createCreep([MOVE], creepName);
    } else {
        console.log(`--- Test case (tick: ${Game.time}) ---`);
        Memory.creeps[creepName].tick = Game.time;

        const gMemory = Memory.creeps[creepName].tick;
        const cMemory = creep.memory.tick;

        if (globalMemoryTick === creepMemoryTick) {
            console.log(`success: ${gMemory} === ${cMemory}`);
        } else {
            console.log(`failed: ${gMemory} !== ${cMemory}`);
        }
    }

    RawMemory.set(JSON.stringify(Memory));
}
```
@RiftLurker
Copy link
Contributor Author

RiftLurker commented Feb 7, 2017

I came up with a solution to set the memory property of Creep, Flag, Room and StructureSpawn, so you can already use RawMemory now.
Even though this hasn't been tested in a larger code base yet, the test case in the Pull Request description above works just fine with it.

@RiftLurker RiftLurker changed the title Make memory property configurable Fix RawMemory Mar 29, 2017
@RiftLurker
Copy link
Contributor Author

RiftLurker commented Mar 31, 2017

@ricochet1k pointed out that my userside fix won't work on the first tick after our runtime resets because the objects are already created before the fix is applied. It would be possible to swap out their prototypes as well, but that adds even more noise to fix something that should already be possible according to documentation.

@artch can we expect any progress on this in the near future? If it requires additional changes on my side let me know.

@CyborgMaster
Copy link

This solution is perfect. Adding the setter to the global Memory object allows you to parse the raw string your way and set things up before the objects start accessing their internal memory properties.

Right now if you try and set it up using RawMemory, Memory will still get parsed and loaded on first access of things like moveTo setting the _move property. But you will most likely overwrite it using the RawMemory.set(JSON.stringify(myMemory)) call at the end of your loop. This causes the built in reusePath option to not work and instead calculate the path every time.

Another way I could fix it in the current API is by overwriting the Creep.prototype.Memory property using my own getters and settings, however they are currently marked as unconfigurable, so I can't override them. This pull request also adds that, allowing the user to solve this however they desire.

@artch, or any other Screeps devs, seeing as how this solution is so simple and effective, is there any reason this hasn't been merged and deployed yet? Is there anything we can do as a community to move this forward?

@CyborgMaster
Copy link

I was hoping I could avoid double parsing the memory (once on my own using RawMemory, and then once automatically when the global Memory object is used) by my just not using moveTo, but I just noticed that there is one other unavoidable place that Memory is used, which is will cause a parse: StructureSpawn.prototype.createCreep. So there is no way to avoid doing the JSON parse on the Memory and implement your own using RawMemory. This appears to make RawMemory useless in its current form. Am I missing something?

@googol
Copy link

googol commented Apr 9, 2017

My old PR #4 is also needed to really use RawMemory (currently there is no way to spawn creeps without using Memory)

@CyborgMaster
Copy link

@googol, I do like your handling of create creep memory better, don't try and set anything if nothing is passed. However, if they adopted @PostCrafter's method of allowing you to set the global memory object, then as long as you set up your custom memory object before hand using RawMemory, it would use your implementation instead of the built in memory.

@RiftLurker
Copy link
Contributor Author

For anyone interested in using RawMemory already, I finally have a solution.

In a discussion on the slack chat, about reusing Memory in subsequent contexts, @ags131 brought up that calling delete global.Memory will remove the property from global entirely.

This will allow us to use a slight variation of the example in the serialization paragraph of Memory.

delete global.Memory
global.Memory = JSON.parse(RawMemory.get()); //on the first access to Memory object
// ...your script
RawMemory.set(JSON.stringify(Memory));

@CyborgMaster
Copy link

@PostCrafter. Exciting! I'll have to try that!

@CyborgMaster
Copy link

I wanted to report back. I have tried @PostCrafter's technique and it worked for me. If I delete Memory from the global object, I am then able to reassign it and it is used throughout script, even in the built-in spawn and move code.

Well done @PostCrafter!

I still think that this PR is a good one to merge because It makes using RawMemory more straight forward. However, I'm not longer desperate for it because we have a work around.

@RiftLurker
Copy link
Contributor Author

@artch With the recent activity regarding PRs, what is the status on this one?

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

Successfully merging this pull request may close these issues.

4 participants