id,summary,reporter,owner,description,type,status,priority,milestone,component,resolution,keywords,cc,phab_field 5387,Rewrite the JS GUI to use object-orientation,elexis,,"**Object-oriented programming advantages, Example simulation components:** The pyrogenesis simulation system is very solid and clarly arranged, because it uses object-oriented programming. Every property an entiy can have is stored in a JS prototype instance. This allows a reader or writer to the codebase to understand a single component without having to understand all the other components (few dependent components only). In contrast, procedural programming has been used in most of the JS GUI part. This means all functions are global, most states are global. So they are represented as peers when in fact they might be entirely unrelated. This is confusing to the one who reads the code for the first time, because he has to infer where the logically related parts are located. But much worse is the fact that the developers who write to the codebase for the first time, even the ones who refactored the code already a hundred times have trouble identifying logical boundaries for the entire codebase (spaghetti code). So when writing to the codebase, it is likely the code becomes even more chaotic (code degradation). So the advantages mentioned for the simulation system only benefit the JS GUI once it's written to use OOP. Patterns / Anti-patterns: * https://en.wikipedia.org/wiki/Object-oriented_programming * https://en.wikipedia.org/wiki/Separation_of_concerns * https://en.wikipedia.org/wiki/Information_hiding * https://en.wikipedia.org/wiki/Software_rot **Example GUI `session/`:** For example consider `Object.properties(global)` of the `session/` folder as of alpha 23: {{{ acceptTerms ACTION_GARRISON ACTION_GUARD ACTION_NONE ACTION_PATROL ACTION_REPAIR addChatMessage addNetworkWarning addResearchToQueue addTrainingByPosition addTrainingToQueue appendSessionCounters armorLevelToPercentageString attackRateDetails AuraTemplates autoCompleteNick backToWork barterOpenCommon barterUpdateCommon basename bicubicInterpolation bodyFont BUTTON_HEIGHT calculateCarriedResources cancelOnLoadGameError cancelPackUnit cancelUpgradeEntity canMoveSelectionIntoFormation changeGameSpeed changePrimarySelectionGroup chatMenuButton checkTerms clampColorValue clearChatMessages clearSelection clearTimeout clickedEntity clone closeChat closeDiplomacy closeMenu closeObjectives closeOpenDialogs closePageHack closeTrade colorDistance coloredText colorFade_attackUnit colorizeAutocompleteHotkey colorizeHotkey colorizePlayernameByGUID colorizePlayernameByID colorizePlayernameHelper colorizePlayernameParameters commaFont comparedModsString compatibilityColor confirmExit controlsPlayer createPanelEntityTooltip cubicInterpolation currIdleClassIndex DamageTypes damageTypesToText deepfreeze deleteGame deleteTooltip DeriveModificationsFromTech DeriveModificationsFromTechnologies DeriveTechnologyRequirements determineAction diplomacyFormatAttackRequestButton diplomacyFormatSpyRequestButton diplomacyFormatStanceButtons diplomacyFormatTributeButtons diplomacyMenuButton diplomacySetupTexts dirname displayGamestateNotifications displayMultiple displayPanelEntities displaySingle doAction DoesModificationApply doublePressTime doublePressTimer DrawTargetMarker END_MENU_POSITION endOfAlert Engine EntityGroups EntityGroupsContainer EntitySelection error escapeText executeCheat executeNetworkCommand exitMenuButton fadeColorTick filesizeToObj filesizeToString findGatherType findGuidForPlayerID findIdleUnit flushTrainingBatch formatAttackMessage formatBarterMessage formatBatchTrainingString formatChatCommand formatDefeatVictoryMessage formatDiplomacyMessage formatLimitString formatPhaseMessage formatPlayerInfo formatTributeMessage formatTributeTooltip g_AdditionalHighlight g_allBuildableEntities g_allTrainableEntities g_AllyEntityCommands g_AlwaysDisplayFriendlyFire g_Ambient g_AttackTypes g_AvailableFormations g_BarterActions g_BarterMultiplier g_BarterResourceSellQuantity g_BarterSell g_BatchSize g_BatchTrainingEntities g_BatchTrainingEntityAllowedCount g_BatchTrainingType g_BiomesDirectory g_BribeButtonsWaiting g_Buddies g_BuddyListDelimiter g_BuddySymbol g_canMoveIntoFormation g_Ceasefire g_ChatAddresseeContext g_ChatCommands g_ChatHistory g_ChatHistoryFilters g_ChatLines g_ChatMessages g_ChatTimeout g_ChatTimers g_Cheats g_CivData g_CivInfo g_ColorFade g_ConfirmExit g_DamageTypes g_DefaultLobbyRating g_DefaultPopulationColor g_DescriptionHighlight g_DevSettings g_DiplomacyColorsToggle g_DiplomacyMessages g_Disconnected g_DisplayedPlayerColors g_DragStart generateSavegameDateString generateSavegameLabel g_EntityCommands g_EntityStates getActionInfo getAllBuildableEntities getAllBuildableEntitiesFromSelection getAllTrainableEntities getAllTrainableEntitiesFromSelection getAllyStatTooltip getArmorTooltip getAttackTooltip getAurasTooltip GetBaseTemplateDataValue getBatchTrainingSize getBiomePreview getBuildingsWhichCanTrainEntity getBuildString getBuildTimeTooltip getCheatsData getCostTypes getCurrentHealthTooltip getDefaultBatchTrainingSize getDescriptionTooltip getDisconnectReason getEntityCostComponentsTooltipString getEntityCostTooltip getEntityLimitAndCount getEntityNames getEntityNamesFormatted getEntityOrHolder GetEntityState getEntityTooltip getGameDescription getGameSpeedChoices getGarrisonTooltip getGatherTooltip getHealerTooltip getHealthTooltip getHistoryTooltip getHotloadData GetIdentityClasses getIdleLandTradersText getIdleShipTradersText getInitColorFadeRGB getLocalizedResourceAmounts getLootTooltip getMapDescriptionAndPreview GetModifiedTemplateDataValue GetMultipleEntityStates getNeededResourcesTooltip getNetworkWarnings getNumberOfRightPanelButtons getPlayerHighlightColor getPopulationBonusTooltip getPreferredEntities getProjectilesTooltip getRepairTimeTooltip getReplayMetadata getRequiredTechnologyTooltip getResourceTrickleTooltip getResourceTypeDisplayName getSavedGameData getSecondsString GetSimState getSpeedTooltip getSplashDamageTooltip getStanceDisplayName getStanceTooltip GetTechModifiedProperty GetTechnologyBasicDataHelper GetTechnologyData GetTechnologyDataHelper GetTemplateData GetTemplateDataHelper getTermsHash getTradingTooltip getTrainingStatus getUsernameList getValidPort getVisibleEntityClassesFormatted GetVisibleIdentityClasses getWallPieceTooltip g_FadeAttackUnit g_FlushTributing g_FollowPlayer g_FormatChatMessage g_FormationsInfo g_FreehandSelection_InputLine g_FreehandSelection_MinLengthOfLine g_FreehandSelection_MinNumberOfUnits g_FreehandSelection_ResolutionInputLineSquared g_GameAttributes g_GameSpeeds g_Groups g_HasRejoined g_HighlightedAlpha g_HotkeyTags g_IdleTraderTextColor g_IsChatAddressee g_IsController g_IsDiplomacyOpen g_IsMenuOpen g_IsNetworked g_IsNetworkedActive g_IsObjectivesOpen g_IsObserver g_IsReplay g_IsTradeOpen g_IsTrainingBlocked g_JumpCameraLast g_JumpCameraPositions g_LastChatAddressee g_LastTickTime g_LoadDescription g_MapPreviewPath g_MapSizes g_MapTypes g_MaxDragDelta g_MaxPlayers g_MaxSelectionSize g_MaxTeams g_MessageBoxBtnFunctions g_MessageBoxCallbackArgs g_MilitaryTypes g_NetMessageTypes g_NetworkCommands g_NetworkWarnings g_NetworkWarningTexts g_NetworkWarningTimeout g_NotificationsTypes g_NumberOfBatches g_OnSelectTab g_PanelEntities g_PanelEntityOrder g_PanelsOrder g_PatrolTargets g_Paused g_PausingClients g_PlayerAssignments g_Players g_PlayerStateMessages g_PluralTranslations g_PluralTranslationsWithContext g_PopulationAlertColor g_PopulationCapacities g_Progress g_RangeTooltipString g_ReplaySelectionData g_ResearchListTop g_ResourceData g_ResourceTitleFont g_Selection g_SelectionPanels g_Settings g_SettingsDirectory g_ShowAllStatusBars g_ShowGuarded g_ShowGuarding g_ShowGUI g_SimState g_SoundNotifications g_SplashDamageTypes g_StartingResources g_StatusBarUpdate g_StatusMessageTypes g_SummarySelectedData g_TabCategoryCount g_TabCategorySelected g_TargetMarker g_TechnologyData g_TemplateData g_Terms g_Time g_TimerID g_Timers g_TooltipTextFormats g_Translations g_TranslationsWithContext g_TutorialMessages g_TutorialNewMessageTags guiToRgbColor g_UnitActions g_unitPanelButtons g_ValidPorts g_VictoryConditions g_VictoryDurations g_ViewedPlayer g_WorkerTypes handleClientsLoadingMessage handleInputAfterGui handleInputBeforeGui handleMinimapEvent handleNetMessages handleNetStatusMessage handleNotifications handlePlayerAssignmentsMessage hasClass hasSameEngineVersion hasSameMods hasSameRestrictionCategory headerFont heapsPermute hideRemaining hideUnitCommands horizontallySpaceObjects hslToRgb init initBarterButtons initChatWindow initGUIObjects initializeMusic initMenu initMusic initPanelEntities initTerms initViewedPlayerDropdown INPUT_BANDBOXING INPUT_BATCHTRAINING INPUT_BUILDING_CLICK INPUT_BUILDING_DRAG INPUT_BUILDING_PLACEMENT INPUT_BUILDING_WALL_CLICK INPUT_BUILDING_WALL_PATHING INPUT_MASSTRIBUTING INPUT_NORMAL INPUT_PRESELECTEDACTION INPUT_SELECTING inputState INPUT_UNIT_POSITION INPUT_UNIT_POSITION_START InterpretTechRequirements INVALID_ENTITY isColorFadeRunning isCompatibleSavegame isPlayerObserver isTranslatableString isUndeletable jumpCamera kickError kickObservers kickPlayer lastIdleClasses lastIdleUnit layoutSelectionMultiple layoutSelectionSingle leaveGame listFiles loadAIBehaviors loadAIDescriptions loadAIDifficulties loadBiomes loadCeasefire loadCivData loadCivFiles loadMapTypes LoadModificationTemplates loadPlayerDefaults loadPopulationCapacities loadSettingsValues loadSettingValuesFile loadTermsAcceptance loadVictoryConditions loadVictoryDuration lobbyDialogButton lockGate log MARGIN markForPluralTranslation markForTranslation markForTranslationWithContext MatchesClassList matchUsername MENU_BOTTOM MENU_SPEED MENU_TOP messageBox messageBoxCallbackFunction ModificationTemplates modsToString mouseIsOverObject mouseX mouseY multiplayerName multiplyEntityCosts music Music NetworkDialogManager NUM_BUTTONS onClientJoin onClientLeave onNetworkOutOfSync onReplayFinished onReplayOutOfSync onSelectionChange onSimulationUpdate onTick onToggleChatWindowExtended OnTrainMouseWheel onWindowResized openChat openDeleteDialog openDialog openDiplomacy openGameSummary openManual openMenu openObjectives openOptions openSave openStrucTree openTerms openTrade openURL optionsMenuButton optionsPageClosed packUnit parseChatAddressee pauseGame pauseMenuButton performAllyCommand performCommand performFormation performGroup performStance pickRandom placementSupport PlacementSupport placeTabButtons playAmbient playerCheck playerDataToStringifiedTeamList playersFinished _playSound positionUnitsFreehandSelectionMouseMove positionUnitsFreehandSelectionMouseUp prepareForDropdown preSelectedAction prevHotkey print raiseAlert randBool randFloat randIntExclusive randIntInclusive randomAngle randomNormal2D reallyDeleteGame recalculateStatusBarDisplay removeDupes removeFiltersFromTemplateName removeFromProductionQueue removeGuard removeOldChatMessage reportDisconnect reportGame resetIdleUnit resignGame resignMenuButton resignQuestion resizeChatWindow resizeDiplomacyDialog resizeTradeDialog resourceIcon resourceNameFirstWord resourceNameWithinSentence Resources resourcesToAlphaMask restartColorFade restoreSavedGameData resumeGame resumeGameAndSaveSummarySelectedData rgbToGuiColor rgbToHsl sameColor saveSettingAndWriteToUserConfig SDL_BUTTON_LEFT SDL_BUTTON_MIDDLE SDL_BUTTON_RIGHT SDLK_LALT SDLK_LCTRL SDLK_LEFTBRACKET SDLK_LSHIFT SDLK_RALT SDLK_RCTRL SDLK_RIGHTBRACKET SDLK_RSHIFT selectAndMoveTo selectNextTab selectPanel selectViewPlayer sendDialogAnswer sendLobbyPlayerlistUpdate setCameraFollow setClientPauseState _setHighlight setJumpCamera setMapPreviewImage _setMotionOverlay setNewTimerFunction setOutcomeIcon setPanelObjectPosition _setStatusBars setStringTags setTimeout setupUnitPanel showTemplateDetails showTemplateViewerOnClickTooltip showTemplateViewerOnRightClickTooltip showTimeWarpMessageBox shuffleArray singleplayerName smoothColorFadeRestart_attackUnit someCanPatrol someGuarding someRallyPoints someUnitAI sortDecreasingDate sortGUIDsByPlayerID sortNameIgnoreCase soundNotification splitRatingFromNick sprintf startBuildingPlacement startColorFade STEP stopColorFade stopUnits storeCivInfoPage stringifiedTeamListToPlayerData submitChatDirectly submitChatInput TechnologyTemplates timeToString toggleChangePerspective toggleConfigBool toggleDeveloperOverlay toggleDiplomacy toggleGameSpeed toggleGUI toggleMenu toggleObjectives togglePause toggleRangeOverlay toggleTrade toggleTutorial TradeGain TradeGainNormalization tradingGainString translate translateAIBehavior translateAIDifficulty translateAIName translateAISettings translateMapSize translateMapTitle translateMapType translateMessageObject translateObjectKeys translatePlural translatePluralWithContext translatePopulationCapacity translateVictoryCondition translateWithContext tryAutoComplete tryPlaceBuilding tryPlaceWall unescapeText unitFilters unitFont unloadAll unloadSelection unloadTemplate UnravelPhases updateAdditionalHighlight updateBandbox updateBarterButtons updateBuildingPlacementPreview updateChatAddressees updateChatHistory updateCinemaPath updateCounters updateCursorAndTooltip updateDebug updateDefaultBatchSize updateDiplomacy updateDiplomacyColorsButton updateDisplayedPlayerColors updateEnabledRangeOverlayTypes updateGameSpeedControl updateGarrisonHealthBar updateGroups updateGUIObjects updateGUIStatusBar updateHotkeyTooltips updateIdleWorkerButton updateMenuPosition updatePanelEntities updatePauseOverlay updatePlayerData updatePlayerDisplay updateResearchDisplay updateSelectionDetails updateTimeNotifications updateTimers updateTopPanel updateTraderTexts updateTutorial updateUnitCommands updateViewedPlayerDropdown upgradeEntity UPGRADING_CHOSEN_OTHER UPGRADING_NOT_STARTED Vector2D Vector2Dprototype Vector3D Vector3Dprototype vsprintf warn }}} These are 682 globals! The reader now has to either use circumstances (functions being located near a file) to identify relations (which is often unreliable and easy to do sub-ideal); or the reader has to read all callstacks of the code to figure out how that works. To get the entire callstack with this spaghetti procedural code, he soon will turn out to have to read half the code base to get what's going on (depending on the most recent state of refactoring and the most recent state of newly introduced spaghetti). To get a visual indication of the code organization issue, one can also draw the graph where each vertex is a JS object and each edge indicates that the object is a property of the other object. This yields a tree / hierarchy (as the codebase avoids circles as far as I know). In the above example the tree would have the `global` root node that owns 682 nodes (or similar). But, hypothetically, the code could be refactored to use about 40 prototypes that each have about 15 properties (states and functions). This way said tree would be much more harmonic, the reader can cut out much larger chunks of understanding of the codebase to understand the area of code relevant to his concern. Even more importantly, the writers to the code base will stop adding procedural code when the entire other codebase uses OOP. This way the spaghetti problem will not only be fixed for the existing codebase but also change the behavior of future developers by giving better example codes that they can orient at. **Precedents rmgen / gamesetup:** In #5322 the same issue was reported for the gamesetup specifically, which in 2015 was the best demonstration of code that works correctly to the user but prevents the readers from understanding it and developers from adding features that are wished since decades. In #5042 the same issue was reported for random maps. The rmgen core was already OOP (matei, historic_bruno...), but many new library functions were added in procedural style later (spahbod, FeXoR, elexis, ...). **Proof of concept / examples:** * The change in Phab:D322 is a good example of how the code looked before and how it looks afterwards (albeit not using prototypes) and it marked a milestone, allowing for iteration over gamesettings that enabled the addition of many more features and made the code easy enough so that new developers had only one hunk of 20 lines of code to read to understand one gamesetting, rather than 10 functions spread throughout the file. * Phab:D1746 demonstrates how new JS GUI code can be implemented using prototypes. **Implementation time consumption and review:** The rmgen cleanup in alpha 23 (nuking duplication #4805, relocating #4804, vector algebra #4845, #4992, OOP #5042) consumed about 8 git refactoring intermediary branches and from September 2017 to April 2018 with no or almost no review and many hours of daily work. The gamesetup cleanups in late 2015 (50 relocation / duplication revmoal commits) and 2016 (Phab:D322 OOP, duplication removal) consumed maybe half a year of working time without reviews. I believe there should be a review of the plan, so that we're all in the same boat and sail towards the same goal. But given that the gamesetup is only one of maybe 20 GUI pages and that the gamesetup refactoring already consumed half a year of unreviewed work, given that the rmgen cleanup consumed about 8 months without review, and given that a mandatory upload or review rule would at the very best introduce a 24 hour delay per commit, not rarely an indefinite delay, this ticket will not be possible to realize with a mandatory review policy, and consume maybe an additional year of lifetime with a mandatory patch upload before commit policy to complete. (Not to mention hundreds of differential revision proposals that only contain the same qualitative change, just for many lines, i.e. would yield a bad signal/noise ratio for Phabricator and distract people from reading revision proposals where people actually need help with non-trivial changes.) So despite unpermitted renames of two variables changes were considered extremely terrible in at least one instance (r20552), I find myself in a situation where I can't satisfy both a review / upload rule and complete this ticket in the remaining time that is provided to me. So I can either offer to comply with any review or upload rule or offer to work on this ticket, possibly offering to upload specific patches before a commit, but not all of them. See the internal discussion about a mirror of this problem https://wildfiregames.com/forum/index.php?/topic/25169-post-a23-commit-frenzy/ which so far concluded in guidelines not being rules. ",defect,new,Should Have,Backlog,UI – Miscellaneous,,,Krinkle,