Current Architecture Projects

PCGen Architecture work

6.3 Projects

Ability Cleanup

Ability Cloning Work

http://jira.pcgen.org/browse/CODE-2465

Tests

Need to develop a significant number of tests to ensure this transition takes place correctly - big risk in duplicate removal not working correctly

MULT:YES STACK:NO items taken multiple times, multiple locations or same location

MULT:YES STACK:YES items taken multiple times, multiple locations or same location

A bunch of this is already in the integration tests (characters), but may want utests to check this (or really itest in the sense of doing tokenmodel tests)

Specifically what needs tests now is bonuses in Abilities - since the bonus system specifically grabs associations, this requires some significant testing to ensure it is correct

In Working Patch

CNAbility Relationship

Factory to build CNAbilities provides the right mix

Simplifies CNAbility usage - build it when you need it (doesn't have to purely be one-way-in one-way-out)

Identity == Required to have separate identities to make removal of items appropriately unique (many facets store source in IdentityHashMap, can't break that yet)

"Useless Adds"

Need an appropriate way of handling these situations and NOT lose information in case the duplicate "useless at the time" is not actually useless

The case is: MyAbility MULT:NO. Add MyAbility|!PRELEVEL:1,Fighter=3. Add MyAbility again at level 2, but the first one is revoked at level 3. How do we know we still have MyAbility?? (we DON'T today - this is a bug) - we must do duplicate removal "later on"

Solution is in having CNAbilityFacet that is MULT aware, then another facet that is stack aware

Disambiguation

Much of this (and how to replace methods) is based on whether items are called as a driver (what made the CHOOSE selection) or the target (the entire ability).

A destructive exploration is here: assocbreakout.23247.patch

The tracker: http://jira.pcgen.org/browse/CODE-2473

Chooseable

Race, Template, CNAbility, Skill, Domain become Chooseable objects

Requires split of ADD/CHOOSE (complete)

Requires Ability cloning cleanup on PlayerCharacter to be completed to the point where the getAssociation related methods can take a Chooseable

Facet Reduction: Abilities are currently handled in two facets and needs to be reduced to one

Future Considerations

Sample Object Removal

Description.getDescription gets a sample object (choosedriver assumption)

BenefitFormatting gets a sample object (choosedriver assumption)

PlayerCharacter.getDescription gets a sample object (choosedriver assumption)

PRExxx source

PREVAR needs source (only reason it is passed through to PRE testers) and that is a problem for CNAbility - want to avoid a special case :/ ... NOT a problem, just have to expose variableSource in some way...

This is actually an abstract issue at this point - due to ADD and Template's FEAT being forced to act as if it is a user selection, the source is not terribly relevant

Source

Handling source (for conditional) and pipeline (disambiguating sources) is tricky, since the CDOMObject source is required to properly evaluate things like PRExxx while the facet source is required in order to ensure that tokens cannot pollute each other

Idea: Have a separate facet store the CDOMObject source (ScopeFacet)

ScopeFacet can then be called by the conditional facet in order to pull the source, thus the conditional facet doens't have to be the first facet in the chain

Associations

May need to move associations out of AssociationSupport and into a Facet

Issue is whether CNAbility is identity or not

Without identity, need to ensure associations are properly attached for the UI

Currently AssociationSupport does not store sources of the associations

Facet is SubScopeFacet with some modifications, specifically to handle source weighting

Quick update does not interact well with cloning, since Ability is used to store associations (change to first Map in SubScopeFacet may help?)

See: assocToFacet.23464.patch

Problem is that restoring items on PCG load is an issue - don't have the original ADDED_FEAT for virtual abilities

This interacts with AssociationListKey.ADD since that is necessary to avoid redoing ADDs (see skills)

Major Issues

Lost Info

Information was destroyed in the PCG - have to assume all ADDs as well as Template's FEAT are "user selections" when that is not true - so removal on a restored PC will not (ever) work correctly

Establish if there is a method of determining the source of an Ability in the PCG file (info was destroyed on save)

LST Editor

Weaknesses

CATEGORY token is not editor friendly

KEY token is not editor friendly

PC Class Level repeatlevel does not properly parse for edit context

Kits are not at all editor friendly

Ability Category is not at all editor friendly

Core View

Getting Abilities into core view requires the Ability Cleanup to be completed, since the system must have facets in the general form expected by perspectives

Get Skill Costs into Perspectives

This has some unique challenges since it is not one object type. Once you pass through certain objects (looking backwards through the event tree) you end up with the PCClass disappearing or other things dropping out, so the Perspective somehow has to display that.

See: skillcostmetafirstpass.23125.patch

Templates

Facet Flow

TemplateInputFacet -> TemplateSelectionFacet -> UnconditionalTemplateFacet -> TemplateFacet

TemplateSelectionFacet -> ChooseDriverFacet

Input knows of a PCTemplate (and forces the CHOOSE if necessary), the TemplateSelectionFacet has a Selection, not just the PCTemplate, and then Unconditional and the model (TemplateFacet) hold just the PCTemplate

Problem: The flow of facets changes object classes multiple times. This is going to be complicated to write into the coreview system and makes templates different from something like languages. I'd rather it be similar.

Problem: TemplateInputFacet, in order to know which Selection<> is related to a Template (so the remove can be symmetric) is "fooling" the TemplateSelectionFacet to say that the source is the original PCTemplate. This allows add/remove symmetry, but the REAL source cannot be stored. Right now this is somewhat academic (as addTemplate() in PlayerCharacter never asks for the source), but in order to get things into the core view system, we want to know that source, so we want to enhance addTemplate on PlayerCharacter, and then store the real source later on (And be able to know that for any Template)

Required Flow

TemplateInputFacet -> TemplateSelectionFacet -> ChooseDriverFacet

TemplateInputFacet -> UnconditionalTemplateFacet -> TemplateFacet

This way the unconditional flow matches how other objects will look, and the selection is solely responsible for the association on the Template. This also means TemplateSelectionFacet becomes an AbstractAssociationFacet (thus more aptly named for its behavior than an AbstractSourcedListFacet)

In the future Domains and Races will change flow to match template

Conditional Templates

These are done in a "pull" fashion, and we want to do more "push". This work is in ConditionalTemplateFacet.

Improve for consistency and for the Core View system. Right now we have visibility to conditionally granted but not conditional (but not granted).

See: CODE-2442.condtemplatename.23102.patch

See: template perspective.23082.patch

http://jira.pcgen.org/browse/CODE-2006

Conditional Skills

Need to get conditional skills broken out into separate objects

SkillsFilter

Remove SkillsFilter to be only for the UI and not impact the core

Issue here is how to do "forward" processing for skills

See: skill filter first pass.23117.patch

Discussion

Dev Proposal is here: https://groups.yahoo.com/neo/groups/pcgen_developers/conversations/messages/3773

_exp formal proposal See: https://groups.yahoo.com/neo/groups/pcgen_experimental/conversations/messages/17226

Facets

Facet work

Bonus MONSKILLPOINTS/LOCKNUMBER needs a wrapping facet on adding a template

Template Facet perspective needs source of the Template to be saved

CompanionMods are still done in a loop in PlayerCharacter - need to put in a facet

Consistency

http://jira.pcgen.org/browse/CODE-2442

Want to reduce Spell Facet uniqueness - see MasterSpells in DataSetID-based

DataSetID based

Related to Context strategic issue

There are certain items that are campaign-related not PC related

Master Usable Skills (see Conditional Skills)

DatasetID / Spells

Master Spells

Make Master Spells use DataSetID facet.

Include a cleanup to make the spells facets sub scope facets

Include CODE-2501 to reduce the memory usage of "unused" spell facets

See: +masterspell.23393.patch

Note Side effect: The spells today list ALL places they COULD be achieved from - the patch above makes them only appear with the sources where the CHARACTER receives them from. This means two things: innate spells don't show up with ANY sourcelist AND spells that do have a sourcelist will only show sourcelists that the PC actually has access to... is this side effect appropriate??

Concurrency

All Facets need to be tested against concurrent modification - so have a recursive removal (which someone could write as an iterator) have it also remove another item of that type - causes CME if not careful

BUGS: Potential

It appears ADD/CHOOSE on MULT:YES abilities no longer respects choices that have already been made; introduced in 6.3

MoveResult.getBaseMovement seems to be wrong as it is ignoring the load

TemplateExportToken feats method seems to be buggy as it ignores the level and HD parameters

PersistenceManager methods are called (including clear, and checking for presence of campaigns). These are no longer used by gui2 and should be removed, see: http://jira.pcgen.org/browse/CODE-1891 http://jira.pcgen.org/browse/CODE-1890 http://jira.pcgen.org/browse/CODE-1889

Description seems to ignore the Feat name when %FEAT is used with a specific Feat ... Need to check if this feature is at all used or valuable. see https://groups.yahoo.com/neo/groups/pcgen_experimental/conversations/messages/17087

Compatibility tokens may be spitting information into ParseResult even if they fail, which is bad as they should silently fail

It looks like an item with BONUS that contains <this> will fail if it is subsequently .COPY='d

Class Loader

SUBSTITUTIONCLASS:A
LEVEL:1
CLASS:B
LEVEL:2
CLASS:a.MOD
LEVEL:3

Does PCClass loader have an issue? with order of ops

Where is LEVEL:3 placed? may be CLASS:B??

We need a complete test pass to determine if all ways to add an Ability respect MULT and STACK. This involves finishing itest/tokenmodel.ability

ABILITY:CAT|NAT|Foo(%LIST)|PRExxx: seems to ignore the PRExxx (pretty much all tokens of this form need to be checked)

It appears that ADD:FEAT|Foo ADD:FEAT|Foo will not work (Foo is only applied once, even if MULT:YES)

PRE house rule checks seem to allow for all kinds of bad behavior. AUTO:LANG|Foo|PRExxx on a FEAT will be passed if FEATPRE rule is enabled - isnt' that wrong??

BONUS tokens that impact the PC that are on EqMods may get applied more than once?? See TEST-98

Looks like STACK:YES item when reselected will re-add the originally selected item (set returned by the chooser is ALL choices, not new ones)

Epic Check: 3.5e Loremaster get bonuses, but if those bonuses are based on class levels, they may be lost since the epic level swap is doing churn in order to get CL correct (if not Loremaster, find something that gets BONUS from an Ability granted by the PCClassLevel)

.FORGET interaction with Unconstructed References. Are false positives being displayed, or should there be some implied method of "FORWARDREF" so that .FORGET doesn't require a whole ton of cleanup when an item is forgotten?

Documentation

Need to doc facets explanation, see https://groups.yahoo.com/neo/groups/pcgen_developers/conversations/messages/3721

Primitive and Qualifier Architecture (Gabriel taking the first pass on the Wiki): https://groups.yahoo.com/neo/groups/pcgen_developers/conversations/messages/3682

Review adding a new token documentation, see https://groups.yahoo.com/neo/groups/pcgen_developers/conversations/messages/3529

Need to document Visibility/View usage

Need to check the items in pcgen.cdom.* to ensure the docs are correct - the facets seem to have taken on a bit of copy/paste error over time as some old references were missed in the docs

Known Bugs

Misc

Error/Enforcement

MULT:NO behaviors

MULT:YES abilities can be given MULT:NO behavior and that probably deserves a warning. The main problem is that if a MULT:YES Ability provides something like ADD:FEAT|Dodge, then Dodge will only be applied once, not the number of times the Ability is taken. That's because ADD is a MULT:NO behavior of the Ability. There are other examples, but the point is that it should be possible to know MULT:NO behaviors and warn on usage

see: detect mult single conflicts.22912.patch

see https://groups.yahoo.com/neo/groups/pcgen_experimental/conversations/messages/17262

Ability Names

Need to have a check on Ability object names, if X is a mult:yes ability then X(a) must be prohibited. While we support this currently in the parsing, there are other later subsystems that end up with ambiguity - specifically if the item appers in ADD:FEAT as an example, the two cannot be distinguished.

See: https://groups.yahoo.com/neo/groups/pcgen_experimental/conversations/messages/17250

Consistency

Bracketed PRE: PCClass/PCClassLevel (ADDDOMAINS) - NEWTAG 431

PCClass SPELLTYPE needs to reject empty and NONE - NEWTAG-430

Epic Checks/BAB

Problem: We expect the code to magically know that in the case of BONUS:CHECKS, the term CL is limited by the classes under the epic limit.

Implementation is ambiguous for certain object. It causes a churn in the objects on the PC.

Counterexample: Should an Ability attached to an epic class level impact the non-epic BAB?

Introduce a method of identifying "non epic levels", something akin to: BONUS:CHECKS|BASE.Reflex|NECL/3

Required Steps

Deprecate BONUS:CHECKS. This forces folks to "look at it" in the transition to 6.3/6.4

Add a BONUS:CHECK as a replacement (which basically does the exact same thing)

Add the term/formula for NECL

Conversion

If a trivial parser can determine that the term CL is used, when BONUS:CHECKS is on a base class (CLASS: line in a Class LST file), then it is converted to BONUS:CHECK using NECL

else it is not automatically converted and requires user intervention.

Refactoring

Type safety

Make SubClassFacet Type Safe, see subclassfacet-typesafe.22718.patch

LoadInfo refers to sizes

http://jira.pcgen.org/browse/CODE-1929

Class Moves

Export Tokens

io.exporttoken.StatToken can be moved to a plugin

io.exporttoken.WeaponhToken can be moved to a plugin

io.exporttoken.TotalToken can be moved to a plugin

Static Method Moves

AttackInfo.getAttackInfo can be moved to an export token

CDOM

pcgen.util.enumeration.Visibility can be promoted to cdom.enumeration

pcgen.util.enumeration.View can be promoted to cdom.enumeration

PC Associations

Eliminate SPECIALTY association

See elim-specialty-assoc.22785.patch

Problem: This breaks Quasvin as redundant information was saved but the redundant data is no longer relevant to the PC: https://groups.yahoo.com/neo/groups/pcgen_developers/conversations/topics/3688

http://jira.pcgen.org/browse/CODE-1908

General Cleanup

Refactoring for Clarity (Gabriel's Suggestions): https://groups.yahoo.com/neo/groups/pcgen_developers/conversations/messages/3782

Would be nice to improve the use of generics in gui2 (they are ignored in many locations where they could be used), see improvegui2generics.23033.patch

Eliminate catching of Exceptions without further messaging of the underlying issue

removeAll (id, source) on SourcedListFacet should be removeAllFromSource

Unit Tests

Cleanup

TokenLibrary.addBonusClass is called widely in test cases (as a static call) and should be consolidated into the testcommon directory in some form

StringBuffers

These probably DO deserve a default size as per the tracker indicating as such

See: stringbuffer size.23000.patch

Strategic Projects

Localization

Type Safety

A number of items need to be made type safe to handle L10N

RacePantheon (Deity)

Incompatible Tokens

A number of tokens are incompatible with L10N because they cannot easily be matched (DESC, BENEFIT, generally any free-form String that is allowed to appear multiple times (Spells will also have a bunch)

These may not matter as much if we use PO files? But it will make the PO files very sensitive to tiny data changes. Is there a compromise?

Also, how would replacements be handled?

General Concepts

We must not make producing a data set materially more complicated than it is today (no requirement to put %L10NNAME% type gunk into data)

We should target an ability to tell us if l10n is complete for any given data set

I need to understand more about L10N than I do today - I need to understand this issue: http://wiki.pcgen.org/Localization

Proposal, see: http://wiki.pcgen.org/Internationalization

PO Files

What tools are available for PO files that make a difference?

Are there any tools to help create PO files in Java since we know what needs to be translated?

What is the advantage if we use PO files vs using a custom format?

Formula Parser

This work is significant since any changes we make in the Formula Parser can't be leveraged by something like the bonus manager until a subsequent revision

Initial work done in 2010 time frame by Nuance to get the Parser up and running

For JEP Replacement

Get new formulas into a set of plugins rather than direct injection into maps

Change operations to be able to handle Number, Number as the parameters and return Number. Allows integer math when possible (Eric is looking at this as an initial project)

Conversion of Terms/Formulas

Convert existing terms/formulas to the new formula parser. A representation of what has been done is in this destructive exploration: formulacompleted.patch

Goal is to avoid the use of pattern matching that is used by the current system. In many cases, there are only a limited number of choices that the pattern match worked to handle, and these can be explicitly built

General Concepts

Formulas

Pluggable

getFunctionName() plus an interface to define which form

loaded like we do the lst tokens today

Three forms

Paren formulas

e.g. today's count() function and the other items in plugin.jepformula

BracketFormula

The COUNT is the bracketfunction

Compatibility with the old COUNT system, e.g. COUNT[CLASSES].

Since the current proposal does not include terms, need to think abotu how COUNT doesn't become a huge IF statement - can we load formula components or some such?

DotEqual Functions

These are actually "derived" from terms as they are not separated by the parser

Things like CL=x or CL.x

PCGen will need a "special" TermResolver to break these out

Validation

Every formula needs to be able to indicate if it is valid or not. This includes both checking the function name (the system will probably do this) as well as all of the arguments of the function

Every formula needs to be able to indicate if it is static (for optimization at runtime)

Parsing does not indicate it is valid. IT could be CNT[CLAZZES], which is not valid because CNT is not a bracketfunction and CLAZZES is not a bracketterm

If not valid, it needs to return WHY it is not valid - much like tokens return ParseResult.FAIL, Formulas need to return a FormulaValidity that indicates success or the problem (pcgen.base.formula.error)

There are no terms

Main Goals

Isolated from PCGen

Isolated via various interfaces

Note pcgen.base.formula.base does NOT have PlayerCharacter as an argument - but womehow it will HAVE to (unfortunately). Probably generics, but the point is that things like count() eventually need to be able to access the PC, and thus the function will have to receive some type of resolution object during evaluate()

Have a system that can be fully unit tested independent of PCGen. Therefore, no references to CDOMObjects or (especially) PlayerCharacter can be in the formula parser. Legal in Formulas since those are plugins

Minimize customization to classes related to the parser - walk the tree when possible. (few svn stored classes in pcgen.base.formula.parse)

Have the ability to detect use of different types of formulas and deprecate them individually

Temporarily compatible with JEP (try new system first, then old)

See: http://wiki.pcgen.org/Formula_Parser_Equip_Vars_Proposal

Equipment

Equipment primitive behavior (e.g. TYPE) can be modified by an EqMod, but that modification must occur in context (Note: Difficult to do this with just a modifier if there is any complexity beyond one level of EqMods)

EqMods can trigger the removal of other EqMods (in context to parent Equipment)

Proposal for Equipment Variables

Equipment needs to be promoted to a "full" object, on the level with PlayerCharacter

Proposal for Equipment to be able to contain other Equipment, needs to handle this in terms of character possession

Does Equipment have a "contains" set such that a PREITEM has to search up the entire "tree" hierarchically? How is that handled? Alternative seems to be that if the PC possesses an item, it must be in the PC's item list. This may end up storing redundant information, however

Is there a way to control the size of Equipment items so that most primitive items do not take up large quantities of memory? (what is the transition threshold and is this done with a strategy of some form?)

Need to finish separating the system so it does not have primary/secondary heads (but numbered)

Equipment would have its own resolution system, facets, etc as necessary

Bonus System Redesign

Proposal posted, see: http://wiki.pcgen.org/Bonus_Subsystem_Design

Breakout of types of Bonuses: http://wiki.pcgen.org/Bonus_Subsystem_Thoughts

Bonuses need to "lose" ownBonuses to get away from ugly behavior

Tokens

Nee to provide special consideration to the tokens we have that are "bonuses in disguise" and whether they should just become pure bonuses

Needs to include a token rebuild that includes better type safety (if they refer to a skill, we need to know if that name is valid or not)

Need some special attention on duplicate bonuses, and deciding what the "long term plan" is - combat has a number of bonuses that are available in multiple ways

Is there work that needs to be considered for bonuses that really aren't? Meaning bonuses that are actually acting as locks. This makes the name "bonus" rather confusing since it is a lock not a bonus

http://jira.pcgen.org/browse/CODE-2502

Export

Token Rebuild proposal

http://jira.pcgen.org/browse/CODE-1904

see: http://wiki.pcgen.org/Dev_Meeting_Log_20091114

Freemarker direct object access

Questions: https://groups.yahoo.com/neo/groups/pcgen_developers/conversations/messages/3795

Provides a way to avoid the complicated token rebuild - focus on core code

Addition abstraction advantages in that input tokens can directly write properties to output without core knowledge if the map functions of freemarker work as per my questions

Looks to be a better solution than the Token Rebuild proposed earlier - less code for us to maintain

http://jira.pcgen.org/browse/CODE-2419

Performance

Bonus System String processing is probably our major performance issue

CollectionToAbilitySelection got a cleanup for infiniteloop but the same code exists in AbilityRefFromChoiceSet

Token Updates

Type Safety

Deity RacePantheon needs type safety

Template/RaceType

Template/Genderlock

Skill/ACheck

Error/Enforcement

Sponsor ImageSmallToken should be deprecated as the code is never used

ChooseLst should probably check that CHOOSE is present on an object if any choice actors are present on an object

CHOOSE:ABILITY and CHOOSE:ABILITYSELECTION the categories need to be a parent category not a pool, see https://groups.yahoo.com/neo/groups/pcgen_experimental/conversations/messages/17129

MULT:YES Abilities can only be directly applied if an association is provided, why is that not true for templates? and how would we actually provide the association?

Need additional checks for .CLEAR tokens - need to ensure .CLEAR applies (utest), and also that .CLEAR and then <blah> on the same line/object results in the correct answer during unparse (itest) [e.g. not caught in fumblerange as of 3/6/14]

Consistency

%LIST Consistency

Deprecate TEMPLATE:x.REMOVE and implement REMOVE:TEMPLATE

Can PreMultParser be moved to a plugin??

CSKILL and CCSKILL use LIST not %LIST

MONCSKILL uses LIST not %LIST

New Token Todo

EquipmentModifier CHOOSE tokens need update to new subtoken format

AbilityCategory should be converted to a CDOMObject and the fields moved to the maps and the tokens updated: Specifically one value here is reuse of ObjectKey's default on getSafe for Visibility

Idealism

Token separation

CHOOSE

Primitive PObject.FEAT= needs to have CHOOSE_INFO reference moved to PlayerCharacter

LoadContext.validateAssociations makes specific reference to CHOOSE_INFO but that is tied to the CHOOSE plugin

PrerequisiteUtilities makes specific reference to CHOOSE_INFO, tied to CHOOSE, move to PlayerCharacter

Enumerations

Delete AbilityCategory.ALL

Delete Nature.ALL

Interfaces

Should LoadContext really be an interface?

Should UnconstructedValidator really be an interface?

Eliminate Token Magic

Template/Subregion has YES

Template/SubRace has YES

Template/Region has YES

PRE/REQ

Design Issues

We have some items that are "prerequisites" (tested when the object is added to the PC) and some that are "requirements" (continually tested). Need to be able to put both on object and distinguish them

Prerequisites are not currently parsed as other items, and as such are not catching typos and otherwise type-safe

There are a number of things that cannot be done in Prerequisites

Limitations to overcome

There are requirements that include things like: 2 Skills of TYPE=Foo of rank 5. The problem we have is that if we list TYPE=Foo twice, then the same skill can match

The processing system we have today has trouble distinguishing between success and accumulation. For things like total number of skill ranks adding up to a number, we have to hack the prerequiste system to handle that, and at that point there are complexities in making that work in PREMULT and also situations that can cause it to fail. The solution is to have Prerequisites return a more complex object that indicates total matches, success, etc. and allow a later decision maker to interpret as necessary.

Right now there is a tit-for-tat system with qualification. Meaning you can write a PRE, and then have a QUALIFY override it and then a PRE:Q: override THAT. Well, that becomes confusing over time and basically ends up in infinite escalation where one item requires more and more text each time. Can you imagine PRE:Q:Q:Q:xxx and QUALIFY:P:P:xxx trying to compete?

http://wiki.pcgen.org/Prerequisite-using_Token_rebuild

FreeMarker

What do we need to have in terms of objects?

Assuming we have a set of objects is there a way we can hide thigns from Freemarker? (annotation) or do we have to build a facade? [looks like facade as best I can tell]

What do we initially want to have available? Certainly NOT the PlayerCharacter object

How do we appropriately wrap the facets for consumption by FreeMarker

Do we want to build a prototype here as well?

Probably need to take OUT the objects that are passed into FreeMarker today so that we don't give anyone the impression we are allowing the use of PlayerCharacter?

Isolation

Core/Output Isolation

Special Ability processing to Strings is happening in PlayerCharacter

http://jira.pcgen.org/browse/CODE-1911

Core/UI Isolation

Stat rolling is happening in PlayerCharacter - isn't this a user like function and thus where should it be?

Events

Rather than subscribing to domainFacet, I think there should be a "display interface" for the core that lets you get a ListFacade<Domain>

It will NOT be an AbstractListFacade<Domain>, but rather one that basically decorates domainFacet and translates the events that way we can keep that translation code localized to that one class (must be event driven per James)

Does NOT duplicate what is in the facades today, just moves it (end up with two "facade sublayers" talking to each other, one that makes "one way in/out" of the core and the other that insulates the UI elements from the core classes

Prototype

Clean up Domain interaction with the UI as a test case for isolation

Make available Domains a list that is processed in the core (allowed perspective)

Also serves as a prototype interface for the Freemarker template use?

See: +DomainWatching.23366.nowork

Awaiting Feedback from James

http://jira.pcgen.org/browse/CODE-1913

Strategic Thoughts

Channels

Long term thoughts when dealing with things like numerical values

Example is something like HANDS on a PC

Modifications that are necessary: Lock, Modify (+-*/), Limit

Need to be able to define a channel, the modify the channel

Need to cleanly address order of operations without having tons of tit-for-tat situations (the PREREQ system of Qualified and :Q: to override that is a tit-for-tat system that has no end

List Channels may also be considered definition, modification

There may need to be a namespace of some form for channels - things like class skill lists for classes could be considered a list, but are in context to a class. Stat value is another one that has a context (the stat)

Spring

Right now we have a lot of hollow objects because Spring only allows single instantiation

How hard would it be to have duplicate - what does it change in XML

What effect would that have on debugging

Effect on cache storage: couldn't use class as key anymore - would need a string of what the facet was to help debug

Other effects?

Strategic Issues

Retroactivity

Retroactive behavior (or lack thereof) is causing issues (and worse, data design changes)

This causes "time travel" problems. What if a level 3 item is retroactive and causes a prereq at level 2 to fail? (or the parent of the retroactive item?)

Context

Globals.getContext is very overused and causes issues with testing

Persistence decodeChoice often requries a call to LoadContext and should be passed in not implicit and grabbed from Globals

This may be fixable with the DataSetID that has been implemented. Many items that have CharID access could use that to get DataSetID and then have a facet that allow conversion of DataSetID to the LoadContext. That would allow a major reduction in overall usage (although how to handle in plugins would be a question since you don't want them directly accessing facets)

Fragility

PCClass.inheritAttributesFrom is fragile, and implies certain behaviors, need a better method of handling this (can overlayCDOMObject work?)

Data Copying and Modification

Need a good strategic method for Variables and Bonuses to handle .COPY= elegantly

Domain Source

DefaultDomainSource

One of the challenges we have in Domains is there is an ability to assign a "default domain source" for the Domain - meaning that it is possible that the intended source is destroyed. It also means that if Domains are added to the "Core View" system that the source may not be correct.

The example as to why a default is a problem is a dual-class Cleric, Druid: If each gets a domain, both of the domains should not be assigned to the default source (whichever class was taken first) So it seems to me that just like some of the other UI elements, we should have a list of the classes (those that allow Domains anyway).

Requirements

Goal: eliminate the default and make Domain sources a full behavior.

We can "pre-select" the class if there is only one that allows domains to avoid another click by the user (this should be a key design criteria)

A class can have itself listed in the Bonus without hardcoding the name (This hopefully has a side effect/benefit on other BONUS tokens of the same nature, and over time we can look at removing some of the code in PCClassKeyChange.changeReferences())

Deprecate BONUS:DOMAIN|NUMBER

The problem is that we have BONUS:DOMAIN|NUMBER If this is always attached to a PCClass, it's not a big deal. If there are bonuses elsewhere, we're pretty much in trouble.

This would be replaced by: BONUS:DOMAIN|x| where x is a class

(1) Compatibility assumes there isn't a class called Number. If we can't make that assumption we can use something like BONUS:DOMAINCOUNT|x|

(2) For any Ability in which the old format is used, we need to add a CHOOSE:CLASS|x and make the BONUS: use %LIST in place of Number. None of the abilities today has another CHOOSE (well, one uses NOCHOICE, but that's no big deal to replace)

(3) Use in the classes themselves becomes a problem. I'd therefore propose we support: %THIS as something referring to itself.

Hardcoding

Magic

TYPE=Natural is magical for Equipment and we need to consider when and how this should drive filtering of objects

Spell isAllowed method hardcodes Potion (defaulting to false) and should be moved to GameMode

PlayerCharacter initializes both INNATE and KNOWN spellbooks by default

PlayerCharacter getSpellRange has hardcoded formulas for spell ranges CLOSE, MEDIUM, etc.

exporttoken.Armor has hardcoded sizes when looking @ additional move

SystemHP.isDNDMassive has hardcoded sizes, also may be buggy because it may be abbreviations that are actually returned?

Constants

Constants.FEAT is specific and should probably be removed

Equipment Locations in Constants need to be extracted to Game Mode

Campaign Enumerations

CampaignID (?) would also be an index in certain enumerations, so those enums (like RaceType) would probably belong in .campaignenum or some such package, and thus use the CampaignID to access the constants. Or perhaps LoadContext needs a .enum as well to hold these simple enumerations (not sure Loadable is worth the effort or has fields that make any sense?)

Major Systems

Primitive/Qualifier

AbilityRefChoiceSet appears to use what is a Primitive in the parenthesis, and has similar/complicated parsing code for the contents. Can/should this be delegated to primitive(s)

Using primitives outside CHOOSE, now part of a proposal, see : https://groups.yahoo.com/neo/groups/pcgen_developers/conversations/messages/3760

GUI2

Speak Languages

pcgen.core.Dataset needs a better method to identify the speak language skill

http://jira.pcgen.org/browse/CODE-2014

Language Chooser facade can use the configured controller for skills too

Need to look at breaking the assumptions in language chooser facade of skill vs other

Visual Quirk: is there a reason we sort items after they are displayed?

Administrative

JIRA

Move PrettyList out of CODE-, see: https://groups.yahoo.com/neo/groups/pcgen_developers/conversations/messages/3765

Recently Completed

March 2014

Leveling Up

add/remove/add class levels causes a major bug with epic characters, need to consider how this is addressed in 6.3

Template Choose/Level

A Template with a CHOOSE which is added by a PCClass level produces an interaction that causes PCGen to dump stack

Problem is interaction

CHOOSE on a Template is fired regardless of the source. For Ability objects, this is only done with directly selected by the user - otherwise it has to be done by passing in an association

ADD: was made properly symmetric in the 6.3 branch, so fixing a tiny piece CODE-3 is literally what caused this regression

When we add a class level, we increment the level, test if it passes the PREREQs, decrement the level, then re-add the level. This is a disaster from an object thrashing perspective and given how CHOOSE is fired on a template is a problem

Solution is non-trivial

Just fixing that the item can be null (the naive fix) will still cause the CHOOSE to be fired twice

Regressing back the CODE-3 fix just kicks the can down the road on the issue

Should probably justify the difference in CHOOSE behavior vs Ability in any solution

Strategic solution would be a better method of doing incrementing levels, but that is a "hard" problem, in that it is complex

On option is to take a page from the ADD: system when a Kit is being applied or a PC is being loaded and set a variable to say "don't process this stuff right now". That can be activated for two different situations: Both the Template Choose/Level problem as well as the epic interaction

Token Abstraction

AbstractBoolean

Global

Descispi

nameispi

Template

Removable

Stat

Rolled

Alignment

valid for deity

validforfollower

Skill

Useuntrained

Exclusive

Size

isdefaultsize

PCClass

IsMonster

memorize

spellbook

allowbaseclass

modtoskills

EqMod

AssignToAll

costdouble

cmod

usemasterskill

campaign

showinmenu

islicensed

ismature

isogl

Ability

Mult

Stack

AbstractString

Template

AppliedName

PCClass

ClassType

ItemCreate

Abb

EqMod

FumbleRange

Equipment

RateOfFire

Deity

Worshippers

Title

Symbol

Appearance

Campaign

Setting

PubNameWeb

PubNameShort

PubNameLong

Help

Genre

Ability

AppliedName

Global

Sortkey

Sorting Associations

PC.getExpandedAssociations needs Collections.sort(ret);

See: CODE-2533

Or is there a better place to ensure this (Description, Aspect, SA, output tokens all do this stuff - and some rely on ChooseInformationUtilities)

Implement association ordering, see https://groups.yahoo.com/neo/groups/pcgen_developers/conversations/messages/3897

April 2014

Conditional Skills Work

Facet Structure

NonExclusiveSkillsFacet: New, gives the non-exclusive skills into the CCSKILL list

MasterToSkillCostFacet: imports the costs on the global objects when a PCClass is added to the PC

SkillFacet (model) needs to meet the following criteria: Has a Rank, has a BONUS:SKILLRANK, non-exclusive and useuntrained

BONUS:SKILLRANK is the main problem in that the current Bonus notification system is not aligned to do an entire bonus type. The current patch does not detect BONUS:SKILLRANK

GUI2 does not properly listen for BONUS:SKILLRANK as it ignores TYPE= situations

Need test fix (Quasvin's PsiCrystal).