On 31.01.2017 10:03, Michael T. Pope wrote:

>> Ok. Seems that Set is used more often (especially in FeatureContainer),
>> therefore duplicate modifiers (incl. all attributes eg. source) are
>> not allowed.
> 
> I can not think of a case where exactly equal Modifiers can coexist.

Okay, then most uses of Set could be lifted to List (except the few
cases where specific modifiers need to be removed from the caches),
correct ?

> However FreeCol
> has a history of surprising its developers with unexpected corner cases.

Can you name some, which I should consider ?

>> Digging a bit further, it really seems the whole duplicity problem
>> (perhaps even the need for the source attribute)
> 
> Source attributes probably started as a GUI feature.

Okay, what exactly are they used for here ?

I see some use in modifier propagation, but that's something I'm
currently trying to get rid of (at least except the spec objects)

>> Matches my observation. Should we then get rid of all sorting outside
>> the GUI ?
> 
> No.  As I said, it is needed when the final value is calculated.

Seems I've got you wrong, and my thougts (operator order) still need
them ...

Do you know any case where that's also required across several object
types ? (IOW: need to sort *after* modifier lists from separate sources
had been cat'ed together, instead of just the source lists.).

OTOH: what about using TreeSet and giving Modifier the appropriate
compareTo() method, so they're always sorted automatically ?

>> Actually, there won't be any need for recalculation, as there wouldn't
>> be any need for any caches (outside the operators themselves), which
>> need to be maintained.
> 
> I do not understand what you are saying here.  AFAICT you were proposing
> a type of compound modifier.  Many(most?) modifiers can change, at least
> in the sense of being added and/or removed. 

Seems I wasn't a bit unclear. My proposal wouldn't change individual
modifiers anymore (and no modifier propagation). Instead the operators
would directly call through to the appropriate objects where the
modifiers originally came from.

For example, Colony could have a StorageCapacity operator, which
operates on the Colony's building list, calling their StorageCapacity
operators.
(okay, maybe not the best example, as we could implement that explictly
in Colony::getStorageCapacity()).

> I take the point that we could stop shovelling features up from the
> buildings to the colony, and indeed that would be cleaner.

Exactly. I'm currently working on that (can post patches if you like).

> The main virtue of the current scheme for warehouses is that at least now
> they are expressible in the spec, rather than being hard coded. 

Yeah, but you need to know *exactly* how the modifier propagation
works - the whole spec is almost its own programming language, which
makes the whole thing pretty hard to understand and maintain.

> IIRC when
> that code was written we were careful to keep the change minimal due
> to major reworking of the production system at the time.

Well, the problem with that is that this moves complexity from the
application to the spec and creates subtle interdependencies.

I'd like to get away from that complexity and just add new logic when
really necessary - with precise documentation.

For example, Colony's goods capacity just depends on the buildings,
which may only add capacity. Until we want some fancy magic like
Tile types adding X per turn, etc., I'd really like to KISS, like that:

https://github.com/oss-qm/freecol/commit/605e481100fe17d3838fe625593d425e95b34932

>>> Also the individual modifiers will still be needed for display purposes
>>> so you will not be saving memory.  
>>
>> Do we really need the individual modifiers, or just groups of them ?
> 
> If they can change, certainly you need them.  You definitely can *not*
> break the listing of applicable modifiers feature.  The users want that
> and demand accuracy.  I do not want to have to fix those bugs again.

Perhaps I was a bit unclear: certainly we've got several user visible
calculation steps, eg. how much defense points certain buildings
contribute, etc. But these always 1:1 mapped to an specific modifier
(individual Modifier instance) or instead all the modifiers from certain
source (eg. specific building) ?


--mtx


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Freecol-developers mailing list
Freecol-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/freecol-developers

Reply via email to