Working Projects - Revision history

Tom Parker: /* What is the solution? */

2018-11-10T22:03:12Z

‎What is the solution?

Tom Parker: /* What is the solution? */

2018-11-10T21:59:24Z

‎What is the solution?

Tom Parker: /* UI Consolidation */

2018-11-10T21:58:43Z

‎UI Consolidation

Tom Parker: /* Filter Consolidation */

2018-11-10T21:54:41Z

‎Filter Consolidation

Tom Parker: /* What is scope of work? */

2018-11-10T21:52:48Z

‎What is scope of work?

Tom Parker at 21:48, 10 November 2018

2018-11-10T21:48:34Z

Tom Parker: /* Eliminate Facades */

2018-11-10T21:08:47Z

‎Eliminate Facades

Tom Parker at 20:13, 10 November 2018

2018-11-10T20:13:33Z

Tom Parker at 19:24, 10 November 2018

2018-11-10T19:24:49Z

Tom Parker: /* What is one possible path? */

2018-11-10T19:17:04Z

‎What is one possible path?

← Older revision		Revision as of 22:03, 10 November 2018
Line 232:		Line 232:
	# For the Avail/Sel panel items, create two new classes, the boxes, glue, addAction and lots of other stuff can be isolated from each of the tabs.		# For the Avail/Sel panel items, create two new classes, the boxes, glue, addAction and lots of other stuff can be isolated from each of the tabs.

−	Note: Each of these new classes probably belongs in pcgen.gui2.util not in tabs.	+	Note: Each of these new classes probably belongs in elsewhere... e.g. pcgen.gui2.util (or in pcgen.gui2.handler, etc) not in tabs.

@@ Line 228: / Line 228: @@
 ==What is the solution?==
-Extract one of them into its own class, make them all shared, slightly expanding generics if necessary.
+# Extract one of them into its own class, make them all shared, slightly expanding generics if necessary.
-For the XInfoHandler classes, you may have to construct it with a few things, but still consolidation is possible
+# For the XInfoHandler classes, you may have to construct it with a few things, but still consolidation is possible
-For the Avail/Sel panel items, create two new classes, the boxes, glue, addAction and lots of other stuff can be isolated from each of the tabs.
+# For the Avail/Sel panel items, create two new classes, the boxes, glue, addAction and lots of other stuff can be isolated from each of the tabs.

@@ Line 219: / Line 219: @@
 The same is true of the XInfoHandler methods in various tabs.
 ==Where do I see this stuff?==
@@ Line 228: / Line 230: @@
 Extract one of them into its own class, make them all shared, slightly expanding generics if necessary.
 For the XInfoHandler classes, you may have to construct it with a few things, but still consolidation is possible

@@ Line 212: / Line 212: @@
 WARNING: For your own sake, do these in small pieces, one target at a time.  You will rapidly find a few (QualifiedTreeCellRenderer comes to mind) that will have a wide ranging impact.
-=Filter Consolidation=
+=UI Consolidation=
 ==What is the situation today?==
 QualifiedFilterHandler is (to within generics) identical between Race, Template, Domain, and potentially other tabs.
 ==Where do I see this stuff?==
@@ Line 225: / Line 227: @@
 Extract one of them into its own class, make them all shared, slightly expanding generics if necessary.

← Older revision		Revision as of 21:52, 10 November 2018
Line 186:		Line 186:

	==What is scope of work?==		==What is scope of work?==
		+
		+	Note: Strictly the new method in Step 1 of Phase 1 is not necessary, although it may help you identify places to look. Longer term it may make sense either to not do the new method and place things directly into the initComponents method, or to do the two phases below and then inline those initializeStaticModel methods at the end. Either way, the target here is the createModels, restoreModels and storeModels methods.

	Phase 1:		Phase 1:

← Older revision		Revision as of 21:48, 10 November 2018
Line 209:		Line 209:
	# It is almost certain you will run into side effects where the Supplier will need to be passed into a constructor at some point - this is the idea, and part of DI.		# It is almost certain you will run into side effects where the Supplier will need to be passed into a constructor at some point - this is the idea, and part of DI.
	WARNING: For your own sake, do these in small pieces, one target at a time. You will rapidly find a few (QualifiedTreeCellRenderer comes to mind) that will have a wide ranging impact.		WARNING: For your own sake, do these in small pieces, one target at a time. You will rapidly find a few (QualifiedTreeCellRenderer comes to mind) that will have a wide ranging impact.
		+
		+	=Filter Consolidation=
		+
		+	==What is the situation today?==
		+
		+	QualifiedFilterHandler is (to within generics) identical between Race, Template, Domain, and potentially other tabs.
		+
		+	==Where do I see this stuff?==
		+
		+	pcgen.gui2.tabs.* QualifiedFilterHandler is, today, an inside class
		+
		+	==What is the solution?==
		+
		+	Extract one of them into its own class, make them all shared, slightly expanding generics if necessary.

← Older revision		Revision as of 21:08, 10 November 2018
Line 115:		Line 115:
	# Delete the WhateverFacade and directly use the object to get information.		# Delete the WhateverFacade and directly use the object to get information.

		+	Good places to start: RaceFacade, DeityFacade, HandsFacade, GenderFacade

	=Create Channels/Delete Facets=		=Create Channels/Delete Facets=

← Older revision		Revision as of 20:13, 10 November 2018
Line 1:		Line 1:
		+
		+	=A COMMENT OVERALL=
		+
		+	It is very helpful for us reviewing the code if you check these in in small pieces and not one massive bulk update. That is one reason why I suggest incremental changes.
		+
	=Token Rebuilds to Interface=		=Token Rebuilds to Interface=

Line 152:		Line 157:

	Note: Step 2 in some cases may be VERY hard. Alignment is an example, where changing Alignment can have side effects. We don't currently have a great design for that scenario, in that checking for qualification of the alignment, checking EX_CLASS, etc. are not behaviors we yet have a design to handle in the formula system.		Note: Step 2 in some cases may be VERY hard. Alignment is an example, where changing Alignment can have side effects. We don't currently have a great design for that scenario, in that checking for qualification of the alignment, checking EX_CLASS, etc. are not behaviors we yet have a design to handle in the formula system.
		+
		+	=Move UI to Dependency Injection model=
		+
		+	==What is the situation today?==
		+
		+	Currently, there are a set of models in the UI that are swapped out for each character. These are done passively on swap, and force many items to have knowledge of CharacterFacade.
		+
		+	==Why do we do this?==
		+
		+	# We only want to build UI items once, and swap the model underneath
		+
		+	This design has a few limitations that we want to get rid of:
		+	# We want to move toward a "dependency injection" model
		+	# We want to lower dependencies on runtime calls up and down a stack (there are classes in a tangle that DI can eliminate)
		+
		+	==Where do I see this stuff?==
		+
		+	# pcgen.gui2.tabs for the most part
		+
		+	==What is the solution?==
		+
		+	Convert these items to dependency injection and eliminate the model swapping effects in the UI.
		+
		+	==Is an example shown today?==
		+
		+	No.
		+
		+	==What is scope of work?==
		+
		+	Phase 1:
		+
		+	# Start with code/src/java/pcgen/gui2/tabs/CharacterInfoTab.java ... Add a method: public void initializeStaticModel(Supplier<CharacterFacade> supplier);
		+	# Add an empty version of the method above to all classes that implement CharacterInfoTab
		+	# pcgen.gui2.tabs.InfoTabbedPane needs to be modified. It:
		+	## Owns the Supplier
		+	## Must call the method above in initComponent()
		+	## May need to initialize the Supplier object and the value of the Supplier in its constructor (depending on how the Supplier is written)
		+	## Note: Null must be a legal return value for the supplier, and it should be initialized to null.
		+
		+	Phase 2:
		+
		+	# For each of the classes where initializeStaticModel was added, look at the following method: public ModelMap createModels(CharacterFacade character)
		+	# For each of the values of a put statement adding to the ModelMap, do the following:
		+	## Evaluate if that subclass needs the CharacterFacade. If so, provide that subclass the supplier in the constructor to that object and alter the class to use the Supplier.
		+	## Delete the put, rather taking the construction of that object (which now uses the supplier) and place it into initializeStaticModel.
		+	## Delete the "install" in restoreModels(...)
		+	## Delete the "uninstall" out of saveModels(...)
		+	## Delete the "uninstall()" method on modified subclass
		+	## Take the items from the "install()" method on the modified subclass and add those to initializeStaticModel (correcting for the difference in location)
		+	# It is almost certain you will run into side effects where the Supplier will need to be passed into a constructor at some point - this is the idea, and part of DI.
		+	WARNING: For your own sake, do these in small pieces, one target at a time. You will rapidly find a few (QualifiedTreeCellRenderer comes to mind) that will have a wide ranging impact.

← Older revision		Revision as of 19:24, 10 November 2018
Line 77:		Line 77:

	You can skip VariableKey, FormulaKey, MapKey for now.		You can skip VariableKey, FormulaKey, MapKey for now.
		+
		+	=Eliminate Facades=
		+
		+	==What is the situation today?==
		+
		+	There are a series of Facades (e.g. RaceFacade) that provide an interface on the core object for use by the UI.
		+
		+	==Why do we do this?==
		+
		+	# Primarily, the facades are isolation of behavior between the core and the UI.
		+
		+	This design has a few limitations that we want to get rid of:
		+	# This forces the UI awareness back into the core, forcing the core to have methods from the Facade Interfaces it doesn't really "want" (and are only there to serve the UI)
		+	# This is very design-specific. Every behavior (Race has Hands) is a game-dependent piece of information that makes PCGen less flexible. While we primarily handle d20, it still is a tight tie that makes it hard to get the UI dynamic to the data.
		+	# This is removing a level of indirection that is necessary preparation for later work that will completely change how we access information in the UI (see below)
		+
		+
		+	==Where do I see this stuff?==
		+
		+	# The facade interfaces are in pcgen.facade.core
		+
		+	==What is the solution?==
		+
		+	For now, this step is to delete the interfaces and directly use the objects. The strategic direction is to use channels. See pcgen.output.channel
		+
		+	==Is this shown today?==
		+
		+	Yes and more. PC Alignment is now controlled through a channel... but that setup is more than this step is asking at the moment.
		+
		+	==What is scope of work?==
		+
		+	# Delete the WhateverFacade and directly use the object to get information.
		+
		+
		+	=Create Channels/Delete Facets=
		+
		+	==What is the situation today?==
		+
		+	There are a series of Facets that store the primary information for the model storing the PC. These are in pcgen.cdom.facet
		+
		+
		+	==Why do we do this?==
		+	# The Facets are small containers for specific sets of information about a PlayerCharacter. These were part of the process of reducing the "god object" behavior of PlayerCharacter.
		+	#: In a larger picture, Facets are going away. There are different types of facets, but in general, the new formula system can take over most of the behaviors of facets. (There are exceptions we will eventually address).
		+
		+	This design has a few limitations that we want to get rid of:
		+	# The facets are still operating in a model where the UI has to go order the core to do things. This is really challenging for things that have interacting behaviors. Gender, for example, is usually PC controlled, but there are some (cursed) magic items that can override it. This really means our solving system (the formula system) needs to be UI- AND object-aware, and we want it to do the resolution of what the current situation is on a PC.
		+	# We want to pass control of ALL of these items that touch the new formula system over to the data. Eventually, the data should know the variable is "PCsAlignment" and then the HTML (or whatever) that displays the PC should be told it's "PCsAlignment". In this way, the variable name from the data can match the HTML and NO code modifications are necessary to change what or how it is displayed. This completes a significant portion of our "separation of duties" to reduce the amount of code change necessary for minor items in game systems, and allows us to focus on larger issues.
		+
		+
		+	==Where do I see this stuff?==
		+
		+	# The facets are in pcgen.cdom.facet, more specifically the ones relevant here are mostly in pcgen.cdom.facet.model
		+
		+	==What is the solution?==
		+
		+	The strategic direction is to use channels. See pcgen.output.channel
		+
		+	==Is this shown today?==
		+
		+	Yes. PC Alignment is now controlled through a channel. You can see some compatibility work (aka temporary code) in pcgen.output.channel.ChannelCompatibility
		+
		+	==What is scope of work?==
		+
		+	Step 1: Move from using the facet to using the new formula system, while Enabling Data to control the channel/variable. This actually takes some interacting work with data.
		+	# We need to set up a system that tells us whether we (code) control the variable name for an item or whether data has asserted IT controls the name. These are called "CodeControls" (see pcgen.cdom.util.CControl for the names, such as ALIGNMENTINPUT)
		+	# If we control the name, we need to create the channel. See SourceFileLoader.defineBuiltinVariables for how alignment works.
		+	# Depending on the complexity, we may need ChannelCompatibility items. Arguably, the Alignment work is not complete, but it is at least functional.
		+	# Delete the facet. There may be other facets using the one you deleted. If so, you will need something like: In PlayerCharacter.doFormulaSetup, you will need soemthing like: VariableChannelFactoryInst.watchChannel(id, "DeityInput", activeSpellsFacet);
		+	If there is a priority [a number] when the listener is set up, you will need to use that as well - please see the other watchChannel method in VariableChannelFactoryInst
		+	# If there is a new format involved (things like DEITY), then you probably need to work with the data team to have that variable defined in all data modes OR you need some method for figuring out where it is/is not legal.
		+
		+	Step 2: Unwind most of CharacterFacade (a bit of a concentrated set of behavior), and reduce the breadth of usage of the pcgen.facade.core.CharacterFacede object within the UI. (.getCharID() usage is good, others to be eliminated)
		+
		+	Note: Step 2 in some cases may be VERY hard. Alignment is an example, where changing Alignment can have side effects. We don't currently have a great design for that scenario, in that checking for qualification of the alignment, checking EX_CLASS, etc. are not behaviors we yet have a design to handle in the formula system.

@@ Line 71: / Line 71: @@
 ## converted to the new load method (not CDOMPrimaryToken)
 ## Have the limit from #3 (limit where it is legal) added
-If they are complicated (FavoredClass), then I'd leave as-is for the moment.
+## If they are complicated (FavoredClass), then I'd leave as-is for the moment.
 # Revert the changes made in #1 and #2