Summary in readme

[rascar-capac]

• the readme file now contains a summary
• the package content have been reorganized
• the methods that were in StringHelper are now extensions

---

This change is 

[rascar-capac]

Reviewable status: 0 of 52 files reviewed, all discussions resolved

---

README.md line 174 at r1 (raw file):

## Helpers

Stateless static functions, shouldn't be extensions because too specific or can't be because not called on an instance

Maybe they could be called Utilities instead, I don't which one you prefer.

[LeanderSimonart]

Reviewed 52 of 52 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @rascar-capac)

---

README.md line 64 at r1 (raw file):

### Animator

Internally caches a name-id map but whats the added value?

the added value of using an id or the added value of the internal map?
The id is for perf reasons the map no idea imo not needed as you should do it yourself

---

README.md line 174 at r1 (raw file):

Previously, rascar-capac (Théo Constant) wrote…

Maybe they could be called Utilities instead, I don't which one you prefer.

I think utilities would be better, helpers are in my opinion more complex and less generic than utilities

[rascar-capac]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @LeanderSimonart)

---

README.md line 64 at r1 (raw file):

Previously, LeanderSimonart wrote…

the added value of using an id or the added value of the internal map?
The id is for perf reasons the map no idea imo not needed as you should do it yourself

bah since we use strings as parameters in those extensions, we still need to compare the string to get the corresponding id in the map. So is that really different from calling the original methods with a string param?
A nicer way to do it would be to generate an enum with the animator parameters and populate the map with the enum as the key and the id as the value. It would also avoid typos in parameters.

[fishingcactus-nathan-istace]

Runtime/Extensions/StringExtensions.cs line 26 at r1 (raw file):

        if( string.IsNullOrEmpty( str ) )
        {
            return null;

Probably nothing to do with that PR, but why does this return null? I would expect that if I call the func on an empty string, it returns an empty string.

[fishingcactus-nathan-istace]

Reviewed 52 of 52 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @LeanderSimonart and @rascar-capac)

[rascar-capac]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @fishingcactus-nathan-istace and @LeanderSimonart)

---

Runtime/Extensions/StringExtensions.cs line 26 at r1 (raw file):

Previously, fishingcactus-nathan-istace (NathanIstace) wrote…

Probably nothing to do with that PR, but why does this return null? I would expect that if I call the func on an empty string, it returns an empty string.

I tried not to stop on every weird code I found while doing this inventory :P Hopefully this and also the publish action will encourage us more to maintain that package!

[fishingcactus-nathan-istace]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @LeanderSimonart)

[lucas-baran]

Reviewed 52 of 52 files at r1, all commit messages.
Reviewable status: all files reviewed, 20 unresolved discussions (waiting on @LeanderSimonart and @rascar-capac)

---

README.md line 0 at r1 (raw file):
Maybe we could add images for various tools in there (maybe not gifs, it might be too much if everything moves inside the README)

---

README.md line 7 at r1 (raw file):

# Runtime scripts summary

| Classes                                | Static? | Mono? | Notes                                                                |

Do you think a Serilizable column make sense? (for all the utility stuff like FloatRange, IntegerRange, Optional, SerializableDictionary, etc)

---

README.md line 11 at r1 (raw file):

| `DebugTimeScaler`                      |         | X     | Scales time and fixed delta time with the exposed value              |
| `EnumDictionary<TEnum, TObject>`       |         |       | Serializable dictionary initialized with all the enum values as keys |
| `FloatRange`                           |         |       | Provides tools to handle min and max float values                    |

Suggestion:

Serializable tool to handle min and max float values   

---

README.md line 15 at r1 (raw file):

| `GUIConsole`                           |         | X     | Runtime console to show Unity logs                                   |
| `GyroscopeHelper`                      |         |       | Gyroscope support                                                    |
| `IntegerRange`                         |         |       | Provides tools to handle min an max integer values                   |

Suggestion:

Serializable tool to handle min an max integer values 

---

README.md line 17 at r1 (raw file):

| `IntegerRange`                         |         |       | Provides tools to handle min an max integer values                   |
| `NetworkStatusChecker`                 | X       |       |                                                                      |
| `Optional`                             |         |       | Wrapper that comes with a serialized bool                            |

or "Serializable wrapper to create optional fields"?

Suggestion:

Serializable wrapper to create optional values

---

README.md line 39 at r1 (raw file):

| Classes                                                                           | Target                              | Notes                                                                                                          |
|-----------------------------------------------------------------------------------|-------------------------------------|----------------------------------------------------------------------------------------------------------------|
| `EnumFlagsAttributes`                                                             | `Enum`                              | When is this useful? The field will already have a special drawer if the enum has the `System.Flags` attribute |

Probably an old script? Idk

---

README.md line 64 at r1 (raw file):

Previously, rascar-capac (Théo Constant) wrote…

bah since we use strings as parameters in those extensions, we still need to compare the string to get the corresponding id in the map. So is that really different from calling the original methods with a string param?
A nicer way to do it would be to generate an enum with the animator parameters and populate the map with the enum as the key and the id as the value. It would also avoid typos in parameters.

the unity animator StringToHash function is most likely using a hashing function like Dictionary so I'm really not sure there is any performance benefit for this.

---

README.md line 174 at r1 (raw file):

Previously, LeanderSimonart wrote…

I think utilities would be better, helpers are in my opinion more complex and less generic than utilities

Agreed

---

README.md line 209 at r1 (raw file):
Description:

Get all types that implement the given type and match the given delegate

---

README.md line 210 at r1 (raw file):
Description:

Get all types that implement the given type and match the given delegate

---

README.md line 211 at r1 (raw file):
Description:

Create exactly one instance of each type that implements the given type and matches the given delegate

---

README.md line 229 at r1 (raw file):
Description for all of them:

Provides readonly access to common collections types with garbage free enumerating and utility functions

---

README.md line 268 at r1 (raw file):
Description:

Adds a context menu action to choose, create and reference an instance of an implemenation of the property type when right clicking on exposed properties deriving from ScriptableObject

---

README.md line 269 at r1 (raw file):
Description:

Utils script to fetch scripts that derive from IPropertyContextMenuCallback and add them automatically to property context menus. This is not meant to be used in project.

---

README.md line 270 at r1 (raw file):
Description:

Adds context menu actions to quickly set values of integer, floating-points and vectors values in inspector

---

README.md line 276 at r1 (raw file):
Description:

Dropdown that allows to search and select an implementation of a given type

---

README.md line 282 at r1 (raw file):

| Classes            | Notes |
|--------------------|-------|
| `TypeDropdownItem` |       |

I think this should go inside the Dropdowns section

---

README.md line 288 at r1 (raw file):
Description:

Allows people to drag and drop scripts inside a scene to quickly create empty game objects with the dragged script attached

---

README.md line 294 at r1 (raw file):
Description:

Use to draw a list of subassets in inspector from a serialized list on scriptable objects. Supports creating new subassets by selecting a specific implementation and delete existing subassets.

---

README.md line 314 at r1 (raw file):
Description:

Utility functions related to the scene hierarchy

---

README.md line 315 at r1 (raw file):
Description:

Utility functions related todrawing things in inspector

[rascar-capac]

I renamed the namespace to FishingCactus.CommonCode, to be more consistent with the other packages. Let me know if you prefer to keep FishingCactus.

Reviewable status: 28 of 122 files reviewed, 2 unresolved discussions (waiting on @fishingcactus-nathan-istace, @LeanderSimonart, and @lucas-baran)

---

README.md line 7 at r1 (raw file):

Previously, lucas-baran (Lucas Baran) wrote…

Do you think a Serilizable column make sense? (for all the utility stuff like FloatRange, IntegerRange, Optional, SerializableDictionary, etc)

Done. I didn’t put an X when it's a Monobehaviour as it would be redundant, but it could make sense to still have them, idk.

---

README.md line 11 at r1 (raw file):

| `DebugTimeScaler`                      |         | X     | Scales time and fixed delta time with the exposed value              |
| `EnumDictionary<TEnum, TObject>`       |         |       | Serializable dictionary initialized with all the enum values as keys |
| `FloatRange`                           |         |       | Provides tools to handle min and max float values                    |

I assume I can ignore those comments since I added the column

---

README.md line 39 at r1 (raw file):

Previously, lucas-baran (Lucas Baran) wrote…

Probably an old script? Idk

That was my guess as well, maybe we should talk about removing old stuff from this package during our next meeting

---

README.md line 64 at r1 (raw file):

Previously, lucas-baran (Lucas Baran) wrote…

the unity animator StringToHash function is most likely using a hashing function like Dictionary so I'm really not sure there is any performance benefit for this.

I'll add a note with my suggestion

---

README.md line 174 at r1 (raw file):

Previously, lucas-baran (Lucas Baran) wrote…

Agreed

Done.

---

README.md line 209 at r1 (raw file):

Previously, lucas-baran (Lucas Baran) wrote…

Description:

Get all types that implement the given type and match the given delegate

Thank you for having taken the time to write those!

---

README.md line 282 at r1 (raw file):

Previously, lucas-baran (Lucas Baran) wrote…

I think this should go inside the Dropdowns section

that's a sub category, I just mirrored the folder hierarchy

---

README.md line at r1 (raw file):

Previously, lucas-baran (Lucas Baran) wrote…

Maybe we could add images for various tools in there (maybe not gifs, it might be too much if everything moves inside the README)

Agreed, especially for editor scripts. I added a TODO

[fishingcactus-nathan-istace]

Editor/ContextMenus/PropertyContextMenus/PropertyContextMenuCallbackFetcher.cs line 5 at r2 (raw file):

using UnityEngine;

namespace FishingCactus.CommonCode

What is the rule for using UnityCommonCode vs CommonCode?

[rascar-capac]

Reviewable status: 28 of 122 files reviewed, 3 unresolved discussions (waiting on @fishingcactus-nathan-istace, @LeanderSimonart, and @lucas-baran)

---

Editor/ContextMenus/PropertyContextMenus/PropertyContextMenuCallbackFetcher.cs line 5 at r2 (raw file):

Previously, fishingcactus-nathan-istace (NathanIstace) wrote…

What is the rule for using UnityCommonCode vs CommonCode?

I think the only remaining place where UnityCommonCode is used is in the repository name. Even the package is called com.fishingcactus.common-code

[fishingcactus-nathan-istace]

Reviewed 94 of 94 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @fishingcactus-nathan-istace, @LeanderSimonart, and @lucas-baran)

[lucas-baran]

Ok for me

Reviewed 89 of 94 files at r2, 7 of 7 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @fishingcactus-nathan-istace, @LeanderSimonart, and @rascar-capac)

---

CHANGELOG.md line 6 at r3 (raw file):

### Added
- Summary in readme
### Updated

don't forget to add that the base namespace has been updated

---

README.md line 7 at r1 (raw file):

Previously, rascar-capac (Théo Constant) wrote…

Done. I didn’t put an X when it's a Monobehaviour as it would be redundant, but it could make sense to still have them, idk.

I agree with you

---

README.md line 282 at r1 (raw file):

Previously, rascar-capac (Théo Constant) wrote…

that's a sub category, I just mirrored the folder hierarchy

Oh yeah right for some reason I don't know how to count up to 3 #

[fishingcactus-nathan-istace]

Editor/ContextMenus/PropertyContextMenus/CreateAssetContextMenu.cs line 9 at r4 (raw file):

using UnityEngine.SceneManagement;

namespace FishingCactus.UnityCommonCode

UnityCommonCode here

[fishingcactus-nathan-istace]

Editor/ContextMenus/PropertyContextMenus/PropertyContextMenuCallbackFetcher.cs line 5 at r2 (raw file):

Previously, rascar-capac (Théo Constant) wrote…

I think the only remaining place where UnityCommonCode is used is in the repository name. Even the package is called com.fishingcactus.common-code

Sorry, I should have been more clear about the other places. I'll add comments where I saw a "UnityCommonCode" package

[fishingcactus-nathan-istace]

Editor/ContextMenus/PropertyContextMenus/IPropertyContextMenuCallback.cs line 4 at r4 (raw file):

using UnityEngine;

namespace FishingCactus.UnityCommonCode

UnityCommonCode here

[fishingcactus-nathan-istace]

Reviewed 6 of 7 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @LeanderSimonart and @rascar-capac)

[rascar-capac]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @fishingcactus-nathan-istace and @LeanderSimonart)

---

CHANGELOG.md line 6 at r3 (raw file):

Previously, lucas-baran (Lucas Baran) wrote…

don't forget to add that the base namespace has been updated

Done.

---

Editor/ContextMenus/PropertyContextMenus/CreateAssetContextMenu.cs line 9 at r4 (raw file):

Previously, fishingcactus-nathan-istace (NathanIstace) wrote…

UnityCommonCode here

Done.

---

Editor/ContextMenus/PropertyContextMenus/IPropertyContextMenuCallback.cs line 4 at r4 (raw file):

Previously, fishingcactus-nathan-istace (NathanIstace) wrote…

UnityCommonCode here

Done.

---

Editor/ContextMenus/PropertyContextMenus/PropertyContextMenuCallbackFetcher.cs line 5 at r2 (raw file):

Previously, fishingcactus-nathan-istace (NathanIstace) wrote…

Sorry, I should have been more clear about the other places. I'll add comments where I saw a "UnityCommonCode" package

Those were probably due to a bad merge with develop :(

[fishingcactus-nathan-istace]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @LeanderSimonart)

[lucas-baran]

Don't forget the merge conflicts

Reviewed 1 of 1 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @LeanderSimonart and @rascar-capac)

---

CHANGELOG.md line 10 at r5 (raw file):

- `StringHelper` methods are now extensions in `StringExtensions`
- Utility script names are more consistent
- Namespaces added/renamed to `FishingCactus.CommonCode`

Suggestion:

- Namespaces updated from `FishingCactus` to `FishingCactus.CommonCode`

[rascar-capac]

Reviewable status: 119 of 122 files reviewed, 1 unresolved discussion (waiting on @fishingcactus-nathan-istace, @LeanderSimonart, and @lucas-baran)

---

CHANGELOG.md line 10 at r5 (raw file):

- `StringHelper` methods are now extensions in `StringExtensions`
- Utility script names are more consistent
- Namespaces added/renamed to `FishingCactus.CommonCode`

done