Draft: Store prototype API

Xenofix

@artch said in Draft: Store prototype API:

Two separate stores energyStore and ghodiumStore each with its own capacity.

With any other property than store it's not a simple unified API anymore. It has to be one store or no store, but we don't win anything if we wrap number fields in xxxStore-objects.

For developing I must disagree with making the energy and energyCapacity deprecated. If you know the structure you are using, then these fields are the preferred way to access these values. The function of store is just to simplify some code for generic transfer/withdraw.

I like the idea with store-properties returning 0. I think what @TuN9aN0 pointed out is an implementation detail here. I would be against Proxy. Store-objects for Extension or Tower could be using a Prototype which is just a view to the Structure without own memory, which has a property defined in the prototype to be const 0 for each resourceType and energy is a getter to the (hopefully not deprecated) energy field of structure.

artch

@tun9an0 Database and API changes are irrelevant. They introduce separate optimizations for engine code and for player code, but they are not related to each other, we might deploy them in separate updates if we want to, but it seems logical to deploy them together. There is no "leaking" in that regard.

As to your compatibility concern, please see the post above.

How many resources were factories going to add again? I sure hope for the Game Object's footprint on the heap it will come through some proxy, otherwise each Structure that can potentially hold something will have this massive hash with 0 values in it...

40 new resource types. But there is no real data for absent resources in a Store. Store.prototype will just contain a getter for every resource type which returns 0 if the underlying DB data object doesn't have the corresponding resource property.

artch

@xenofix said in Draft: Store prototype API:

For developing I must disagree with making the energy and energyCapacity deprecated. If you know the structure you are using, then these fields are the preferred way to access these values.

I don't get it, why exactly is extension.energy so much more convenient than extension.store.energy that we have to keep this ugly duplication forever?

TuN9aN0

@artch Ok well if it's not for infrastructure concerns, then my feedback is I do not need this change, I think it's currently fine. In my view it actually introduces a leaky abstraction. I don't need unified access to things that are essentially different logically. I would not model it this way.

artch

@tun9an0 Very well then, thanks for your feedback! Let's see what other people will say, since this inconsistency and ambiguity in how resources are stored is being long complained by many players.

Xenofix

To me, carry is inconsistent. I would want to have it be named store instead. And a Store-object for general purpose stores is also very welcomed.

But for every other non-general-purpose structure this is only a little nice-to-have feature to simplify withdraw/transfer. If dealing with structure-specific code, I would prefer to access energy and other fields directly. If it's general purpose refiller/robbery code, then I would go for store.

Important to be is that such a change does not have a negative performance impact.

artch

@xenofix

If dealing with structure-specific code, I would prefer to access energy and other fields directly.

Still can't seem to understand what's the difference between addressing .energy and .store.energy?

As to performance, CPU-wise it will not be affected, only heap-wise concern is valid here, and we can simply raise the heap limit on deploying this change.

ags131

As both a mod developer and player I'm exited about this update, for modding, it was always annoying not being able to iterate over resources in a simple way. As a player, the entire store/carry/energyCapacity/carryCapacity mess was annoying, in some cases, I actually wrapped all those to make them match, very similar to how this is designed. It might seem a bit more clunky for stuff with only one resource, but simplifies more than enough other code to make it worth it.

For those wanting to keep energy/energyCapacity, they can easily do a simple prototype extension in their code. Object.defineProperty(StructureExtension.prototype, 'energy', { get() { return this.store.energy } }) Object.defineProperty(StructureExtension.prototype, 'energyCapacity', { get() { return this.store.capacity } })

wtfrank

@ags131 yeah I've had to abstract around energy, store etc in various places (e.g. if (instanceof_energy_structure(struc)) { } else if (instanceof_store_structure(struc)) { } )

Before I used typescript and enforced this at "compile-time", I remember fixing various bugs where I tried to treat an extension as if it had a store, or some other structure the other way around.

I will be glad to have a unified interface, even though I'll have to at some point rework some of my code.

It will make the game slightly less complex for new players.

Smokeman

From my point of view, I like and welcome this change, it will allow me to streamline my creep states so they can exit faster (Thus saving CPU.) I am concerned that heap use may increase, but that's Artch's problem... and in reality, any heap issue impacts everyone, mostly heavy users and users past the 300 CPU limit first, so I don't have a problem with that, despite that I'm breathing down the 300 CPU limit's neck. At the end of the day, it's pretty fair.

Two potential issues. First, the obvious one: Nukers with their 5000 ghodium. Two stores? Ewww! Don't make them all the same except for this one weird bird. Applying a limit object with, for example: {G:5000,energy:250000} to it is the best solution, other structures would have null there to short circuit any checks.

Then the 800 pound Gorilla: LABS. Labs are incompatible with a Store object unless you dynamically reset the limit object on the fly. Sure, the capacity remains 5000, but it can change from (for example.) {UO:3000,energy:2000} to {energy:2000} to {KH:3000,energy:2000}. It can have only one non-energy resource as it uses that to determine what it's doing.

eduter

I like the idea of a unified way of withdrawing / transferring resources, specially with getters that return 0 instead of undefined. I already have something like that in my code. I would have to keep it, though, since mine also supports pickup and drop. I'm not sure this change would benefit me specifically, but I see its value for general purpose code. The only things I don't like about this proposal are including nuker into this, which seems like a stretch and doesn't really add any value, and deprecating the old API. Forcing everybody to change their code, making it slightly more convoluted or it will eventually break, will just make people angry. I see no benefit in that. Keep both and everybody is happy.

W4rl0ck

I also like the new structure .. I already had added prototypes for some of the features, like a cached storeSum and carrySum and having it official is a plus. If the api is decided I'd like to add a polyfill so I can already start to use the new store object

Xenofix

I also welcome the API change but without deprecation of energy. If energy must be deprecated, then please make sure they are configurable to deactivate any warnings.

I think professional players already know how to overwrite prototypes and have aliases and getters for their needs already. Everyone can already model a unified store, and properties for energy and whatever. Keeping that in mind, the API change is mainly for newbies and those who cannot change prototypes using typescript or have found ugly ways around the inconsistent API which they like to simplify.

As a newbie I did often access creep.energy directly instead of creep.carry.energy. The same for storages, it was just so annoying that I always have to access either carry.energy or store.energy or energy when at first all I care about is just simple energy. The tower has no store, the creep has no energy and no store but carry instead; just not consistent. But I can make it consistent with prototypes. So yes, this change would not really help me, but it will help other newbies. One store for all kind of objects is a great change for the API.

About the database change: Please just choose the most efficient model.

I think every high-level player at 300 cpu cap and those who are looking forward to be one agrees with me that performance is more important than convenience. The database is hidden from us, so I prefer an efficient database model over a simple engine code. I have no idea if your proposed database change will have a positive or negative impact on overall performance, but I just fear that it's a negative impact because it looks more complex than before. In my eyes, the API and the database change are two different changes.

In the end these proposed convenience changes should meet the following requirements:

one store for every relevant object to allow a unified way to access energy and resources for convenience.
It shall not increase overall tick rate, better decrease it.
It shall not significantly decrease the free memory we can use
It shall not increase our cpu consumption, better decrease it.

If that's all met, then I believe every single player in the community will be happy about this change.

artch

@xenofix

I have no idea if your proposed database change will have a positive or negative impact on overall performance, but I just fear that it's a negative impact because it looks more complex than before.

It will be positive. Look at this engine code:

exports.calcResources = function(object) {
    return _.sum(C.RESOURCES_ALL, i => typeof object[i] == 'object' ? object[i].amount : (object[i] || 0));
};

It will benefit greatly from this database change:

exports.calcResources = function(object) {
    return _.sum(object.store);
};

artch

@eduter

Keep both and everybody is happy.

This is exactly how legacy monsters are born

artch

@smokeman

Then the 800 pound Gorilla: LABS. Labs are incompatible with a Store object unless you dynamically reset the limit object on the fly. Sure, the capacity remains 5000, but it can change from (for example.) {UO:3000,energy:2000} to {energy:2000} to {KH:3000,energy:2000}. It can have only one non-energy resource as it uses that to determine what it's doing.

It will be ruled out by means of supportedResources that is set according to circumstances of the given lab.

black0441

For the nuker, you could have it charge instead of store-so the ghodium is lost when added to the nuker, instead of when it's used.

Labs are trickier. Maybe add a .store.maxMineralType/.store.maxMineralAmount that returns the mineral with the highest count (not including energy, power, etc)? That way it would always work like .mineralType/.mineralAmount does now on labs, plus it would be useful for storage, terminals and creeps too.

Xenofix

Maybe a non-general-purpose and multiple-resource structure needs capacityFor of e.g. {G:5000,energy:250000}.

capacity, getUsedCapacity() and getFreeCapacity() just makes sense for general-purpose (Storage, ...) and single-resource structures (Tower, Extension, ...). For all structures with mixed resources (powerSpawn, lab, nuke, ...) there needs to be more methods. Maybe getCapacityFor(resourceType), getUsedCapacityFor(resourceType), getFreeCapacityFor(resourceType). These can be implemented for general-purpose Store, too. So that the API for both types is the same.

QGazQ

I must say I'm for this change, and after a period of transition removing the old ones. It did confuse me a lot originally, and though I now have prototypes that handle most of it removing them will simplify things.

As others have mentioned there needs to be some way to define the limits for Labs and Nuker, as others have mentioned a single store object would be my preference with some kind of per resource limit field. The other "important" thing I do a lot is to check what resource type is in a lab, at the moment this is a simple .mineralType. I'd rather not have to iterate over the store / supportedResources and ignore energy to be able to get it in the new way of doing things (though if it were the only way I could add a prototype of .mineralType which did it and cached the result).

deft-code

I really like the unification of carry, store and energy as well as the inclusion of total and free methods.

The beauty of unified store is marred by StructureNuker and StructureLab, we need a little more consideration here.

I think everyone dislikes separate properties. I think @Xenofix's idea of extending the Store interface deal with the two oddballs is best.

How about extending the getUsedCapacity and getFreeCapacity to accept an optional resource type argument and adding a getTotalCapacity.

That way I can easily write uniform transfer and withdraw code by always including the resource type.

nuker.store.getTotalCapacity(RESOURCE_GHODIUM) => 5000
nuker.store.getTotalCapacity(RESOURCE_ENERGY) => 300000
nuker.store.getTotalCapacity() => 305000 // or 0 or whatever make sense
creep.store.getUsedCapacity(/*anything*/) => the same value

// look how nice labs work out.
lab.store.getUsedCapacity(RESOURCE_UTRIUM) => 1000
lab.store.getTotalCapacity(RESOURCE_UTRIUM) => 3000
lab.store.getTotalCapacity(RESOURCE_KEANIUM) => 0

Store.capacity === Store.getTotalCapacity()
Store[RESOURCE_ZYNTHIUM] === Store.getUsedCapacity(RESOURCE_ZYNTHIUM)

I would never need to use Store.supportedResources since getTotalCapacity answers that same question, and for the most part supportedResource is statically known based on structureType.

I think we'll still want a .mineralType member on the lab, but that could just be implemented with getUsedCapacity.