Current Architecture Projects

PCGen Architecture work

6.3 Projects

Ability Cleanup

Ability Cloning Work

Methods to Remove

getAbilityNature ~ 10228 used by the facade (does the facade need to think of a CNAbility as the AbilityFacade?)

getAbilityCategory ~ 10221 used by CATEGORY property in output tokens ... Why is .CATEGORY legal when it is ambiguous? see https://groups.yahoo.com/neo/groups/pcgen_experimental/conversations/messages/17128

getFullAbilitySet ~ 9135 used by gui2 to initialize temp bonuses - why isn't getBonusContainer used?

getAbilityList ~ 10207 (widely used at this point)

getAggregateAbilityListNoDupes ~9024 used in getCDOMObjectList (then temp bonuses & spells)

getAggregateAbilityList ~8995 used in getAbilityKeyed, and prerequisites ...Why can PREABILITY be used without a CATEGORY? What is the use case there? see https://groups.yahoo.com/neo/groups/pcgen_experimental/conversations/messages/17135

getAggregateFeatList ~ 8974 used in bonuses, see TEST-97, see removeFeatAggregate.23264.TEST-97.patch

hasVisibleAbility ~ 9062 used in AbilityCategory's isVisible ... Why are Ability Categories visible based on export and not display? That doesn't make a ton of sense - note to James

getAllAbilities ~ 8507 used in PCG I/O

getAbilityKeyed ~ 8924 used in PCG I/O

getAutomaticAbilityKeyed ~ 8912 used in PCG I/O

getSelecctedAbilities ~ 8966 needs replacement by CNAbility usage

hasUserVirtualAbility ~10134 directly tied to cloning, will be removed as a side effect

hasAbilityKeyed ~8907

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

getAssociation breakout cannot yet be completed, see: assocbreakout.23247.patch

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

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

Eliminate NEEDS_SAVING for Virtual Abilities (store in a different facet if necessary)

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

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. Proposal is here: https://groups.yahoo.com/neo/groups/pcgen_developers/conversations/messages/3773

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

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

SkillsOutputOrder enum

The switch where this is used makes it fragile if a new filter is added

Should be made into an "intelligent" enumeration rather than just being values and those values used in a switch to get behavior

See: skill filter step1.23121.patch

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

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

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

Known Bugs

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

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

FOP0205HandlerImpl seems to be unused, see https://groups.yahoo.com/neo/groups/pcgen_developers/conversations/messages/3833

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

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

SkillBuffers

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

Remaining TODOs

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

A=B get converted to two different objects: A becomes a DotEqualFormula and then a scoped term, B being the scope. CL is implemented as an example in the sandbox

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

The string CLASSES is called as a BracketTerm in the scope of COUNT

Calling as a term prevents COUNT from having to implement a huge IF statement and allows expansion in a separate plugin

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

DotEqual Functions

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

Things like CL=x or CL.x

Strange "trace"

(1) term system separates out CL=x into two items, CL is a EqualFormula name, x is the argument

(2) CL EqualFormula instance is loaded and passed the argument

(3) CL EqualFormula calls back to the FormulaScope and asks it to resolve a term "in scope" of the argument

(4) The "CL" term with a scope of PCClass.class <x> is called for resolution

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)

Terms

Individual items that can be "solved"

Can be global, e.g. HANDS

Scope

Scope is provided by the object from which the formula is called (what is called the source today)

Hope to move scope to a more active system that does not depend on String Scope

Can have a scope, such as "CL" having to be resolved for a class.

Resolution

Inconsistent with how formulas are handled. That is a bit confusing, but given that formulas need validation, evaluation, static check, it may be better NOT to isolate them the same way as terms

DO take PlayerCharacter as an argument to resolve. This is managed by the FormulaScope implementation which is then allowed to be PlayerCharacter-aware (and note the delegation path is to request term resolution from the scope, NOT to get the Term from the scope and then resolve it

Main Goals

Isolated from PCGen

Isolated via FormulaScope interface

Note pcgen.base.formula.base does NOT have PlayerCharacter as an argument - it should NOT.

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 and Terms 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)

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

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

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

Consistency

%LIST Consistency

Bracketed PRE: PCClass/PCClassLevel (ADDDOMAINS, Domain)

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

Abstraction

AbstractFormula

Stat

Statmod

PCClass

startskillpts

crformula

race

leveladjustment

eqMod

cost

costpre

Equipment

pageuse

global

select

template

leveladjustment

AbstractBoolean

Skill

Useuntrained

Exclusive

Template

Removable

Stat

Rolled

Alignment

valid for deity

validforfollower

Size

isdefaultsize

Global

Descispi

nameispi

EqMod

AssignToAll

costdouble

Ability

Mult

Stack

PCClass

IsMonster

memorize

spellbook

allowbaseclass

modtoskills

campaign

showinmenu

islicensed

ismature

isogl

cmod

usemasterskill

AbstractString

Template

AppliedName

PCClass

ClassType

ItemCreate

SpellType

Does this need to be deprecated?

Abb

EqMod

FumbleRange

Equipment

RateOfFire

FumbleRange

Deity

Worshippers

Title

Symbol

Appearance

Campaign

Setting

PubNameWeb

PubNameShort

PubNameLong

Help

Genre

Ability

AppliedName

Global

Sortkey

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

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?

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)

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

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

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

Missing Capabilities

WEAPONBONUS is missing from the GUI2

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