Fixes/Refactoring #172
Open
Fixes/Refactoring #172
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- After later reading ``LoadAllBundles`` and seeing that the ``GetFiles`` callback is exactly the same as the one prior to calling the method, I just made the method receive it as argument, turning two callbacks to one.
- Loop iteration now uses switch case statement instead of explicit value checks in the mod's merge settings. - Loop iteration stops once a matching extended mod is found.
- Changed series of if statements to switch case statement to pick the collection. - Loop iteration is stopped once a dungeon flow is found.
- Changed check in if statement to reduce identation for readibility only. - Loop iteration in ``extendedLevel.SelectableLevel.spawnableScrap`` is stopped once it has found an already injected item.
- Changed series of if statements to switch case statement to pick the collection. - Loop iteration is stopped once a dungeon flow is found.
- Loop iteration stops once a level is found
- Changed from two iterations to one. - Changed series of if statements to switch case statement. - Changed logic of removing the level to include the check of the route being hidden (previously it would only consider it if there was no filter selected, I don't think a filter should decide to make this visible suddenly)
- Removed if statement related to replacing the content between the last newline and to the end of the string with nothing. - Tried making it actually replace the string but it didn't produce a readable result so I don't believe the logic was correct either. - Changed string construction to use interpolation, more readable and less confusing. - Used switch case statement to define the contents of ``tagString``
- Removed the ``ServerRpc`` attribute from the method. The reasoning being that this method, while it is meant to be executed only by the server, it doesn't seem desired to be able to be called by clients as all callbacks to this method has either performed a ``IsServer`` check or it is called inside a ``ServerRpc`` attributed method. I believe this would solve the issue with people failing to leave the lobby due to this method trying to be called on clients while they are leaving at the same time.
- Added null check on ``BuyableVehicle.secondaryPrefab`` to avoid registering a null prefab. This is safe to do because ``ItemDropship`` checks if the selected ``BuyableVehicle`` has a non-null ``secondaryPrefab`` to spawn it in ``DeliverVehicleOnServer``.
- ``MatchingProperties`` takes a type parameter which is ``ExtendedContent``. This means: - ``LevelMatchingProperties`` is a ``MatchingProperties<ExtendedLevel>`` - ``DungeonMatchingProperties`` is a ``MatchingProperties<ExtendedDungeonFlow>`` - Turned ``GetDynamicRarity()`` to virtual which takes as parameter the extended content. This is then overriden with respective derived classes to include additional rarities. - Changed ``RegisterExtendedContent`` to not have a series of if statements checking the explicit type of the ``ExtendedContent`` parameter and instead just made a ``Register`` method which is overriden by all its derived classes and it calls this method instead, removing the need for explicit type checks. - Same logic with ``UnregisterExtendedContent``
This reverts commit 95026cc.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LoadAllBundlesRequest: ChangedLoadAllBundlesmethod signature to accept as parameter the collection of strings which represent the files retrieved from theDirectory.GetFilescallback, reducing the number of callbacks ofDirectory.GetFilesfrom two to one.OnInitialBundlesProcessed: Fixed the loop always skipping the remaining elements ofnewGroup.GetAssetBundleInfos()after the first iteration.RegisterExtendedMod: Changed the loop iteration to stop once it has a found matching extended mod and used switch case statement forextendedMod.ModMergeSettingto know what needs to be checked for matching.TryGetExtendedDungeonFlow: Used switch case statement to define what collection to use for iteration and stopped the iteration once an extended dungeonflow has been found.InjectCustomItemsIntoLevelViaDynamicRarity: Loop iteration onextendedLevel.SelectableLevel.spawnableScrapis stopped once it has found an already injected item.TryGetExtendedLevel: Used switch case statement to define the collection for iteration and stopped the iteration loop once an extended level has been found.GetExtendedLevel: Stopped the iteration loop once an extended level has been found.FilterMoonsCataloguePage: Merged two iterations into one, used switch case statement to define the criteria to wether it is shown or not in the catalogue. Also changed the filter to include the check of wether the moon is hidden or not (any defined filter would ignore the route hidden check)GetMoonCatalogDisplayListings: Removed if statement related to string replacement as it was both not being used in the end result of the message and it didn't seem to produce the desired result as it would make the catalogue unreadable. Changed string construction to use interpolation instead for easier understanding. Used switch case statement to define the contents oftagStringNetworkRegisterCustomContent: Added null check onBuyableVehicle.secondaryPrefabto avoid registering a null prefab. This is safe to do becauseItemDropshipchecks if the selectedBuyableVehiclehas a non-nullsecondaryPrefabto spawn it inDeliverVehicleOnServer. (fixes #173)MatchingPropertiestakes a type parameter which isExtendedContent. This means:LevelMatchingPropertiesis aMatchingProperties<ExtendedLevel>DungeonMatchingPropertiesis aMatchingProperties<ExtendedDungeonFlow>Turned
GetDynamicRarity()to virtual which takes as parameter the extended content. This is then overriden with respective derived classes to include additional rarities.Changed
RegisterExtendedContentto not have a series of if statements checking the explicit type of theExtendedContentparameter and instead just made aRegistermethod which is overriden by all its derived classes and it calls this method instead, removing the need for explicit type checks.Same logic with
UnregisterExtendedContent