|
| 1 | +# How to contribute feedback and ideas |
| 2 | + |
| 3 | +To participate in conversations, please create a GitHub Account and engage in existing or new reports. Please act |
| 4 | +professional by maintaining a friendly and helpful attitude and refraining from using sarcasm and other disrespectful |
| 5 | +language. |
| 6 | + |
| 7 | +When formulating feedback and ideas, it is preferable to bring forward as much relevant information as possible to help |
| 8 | +the developers. |
| 9 | + |
| 10 | +## Examples |
| 11 | + |
| 12 | +### Example study 1 |
| 13 | + |
| 14 | +A developer presents a change where the Gamma damage of the GLA Toxin Tunnel Defender is increased from 52 to 60, so |
| 15 | +that its damage is consistent with the GLA Toxin Tunnel Defender from the campaign mission. This affects multiplayer. |
| 16 | + |
| 17 | +We post feedback to this proposal. |
| 18 | + |
| 19 | +Bad 1: |
| 20 | + |
| 21 | +> This is a silly change. |
| 22 | +
|
| 23 | +Bad 2: |
| 24 | + |
| 25 | +> I do not like this. Looks unnecessary. |
| 26 | +
|
| 27 | +Good 1: |
| 28 | + |
| 29 | +> This change looks suspicious. Could you please explain why the increased damage is desired? |
| 30 | +
|
| 31 | +Good 2: |
| 32 | + |
| 33 | +> This change is likely undesired, because the Gamma damage of the GLA Toxin Tunnel Defender is already 30% better than |
| 34 | +> the rocket damage of all other faction rocket men, except that of the laser guided missile of the USA Missile Defender |
| 35 | +> (+100% damage). The Toxin General is already one of the strongest factions in the game and raising the Gamma damage of |
| 36 | +> the rockets will elevate its strengths unnecessarily. |
| 37 | +
|
| 38 | +### Example study 2 |
| 39 | + |
| 40 | +We look over several different changes and do not understand their purpose. We would like to ask for clarification |
| 41 | +before we go into more detailed feedback. |
| 42 | + |
| 43 | +Bad: |
| 44 | + |
| 45 | +> I looked at several changes and half of them made no sense. No idea what you guys are doing. |
| 46 | +
|
| 47 | +Good: |
| 48 | + |
| 49 | +> I looked at these changes ---inserted list of changes--- and it is not clear to me what their purpose is. Some of them |
| 50 | +> appear to be missing a rationale. Could you please update them so I can re-review them? |
| 51 | +
|
| 52 | +## How to contribute as developer |
| 53 | + |
| 54 | +To make content contributions, aka Pull Requests, either fork this repository and clone that fork, or just clone this |
| 55 | +repository if you are invited as a Contributor of the TheSuperHackers. |
| 56 | + |
| 57 | +Various Generals modding documents are [located here](https://github.com/TheSuperHackers/GeneralsDocuments). |
| 58 | + |
| 59 | +Various Generals modding tools are [located here](https://github.com/TheSuperHackers/GeneralsTools). |
| 60 | + |
| 61 | +## Change Documentation |
| 62 | + |
| 63 | +Every change for the game needs to be documented. |
| 64 | + |
| 65 | +All documentation ideally is written in the present tense, and not the past. |
| 66 | + |
| 67 | +Good: |
| 68 | + |
| 69 | +> Fixes particle effect of USA Missile Defender |
| 70 | +
|
| 71 | +Bad: |
| 72 | + |
| 73 | +> Fixed particle effect of USA Missile Defender |
| 74 | +
|
| 75 | +When a text refers to a faction unit, structure, upgrade or similar, then the unit should be worded without any |
| 76 | +abbrevations and should be prefixed with the faction name. Valid faction names are USA, China, GLA, Boss, Civilian. |
| 77 | +Subfaction names can be appended too, for example GLA Stealth. |
| 78 | + |
| 79 | +Good: |
| 80 | + |
| 81 | +> Fixes particle effect of USA Missile Defender |
| 82 | +
|
| 83 | +Bad: |
| 84 | + |
| 85 | +> Fixes particle effect of MD |
| 86 | +
|
| 87 | +### 1. Script Changes and Documentation |
| 88 | + |
| 89 | +Changes ideally are concise and only touch the lines of code that the change relates to. Any changes made to INI and STR |
| 90 | +files need to be accompanied by a comment where the change is made. The comment can be inlined or put at the begin of |
| 91 | +the changed block. It must be clear from the change description what has changed. |
| 92 | + |
| 93 | +The expected comment format is |
| 94 | + |
| 95 | +```text |
| 96 | +; Patch104p @keyword author DD/MM/YYYY A meaningful description for this change. |
| 97 | +``` |
| 98 | + |
| 99 | +The `Patch104p` word and `@keyword` are mandatory. `author` and date can be omitted when preferred. |
| 100 | + |
| 101 | +| Keyword | Use-case | |
| 102 | +|--------------|-------------------------------------------------------------| |
| 103 | +| @bugfix | Fixing a bug | |
| 104 | +| @feature | Adding something new | |
| 105 | +| @fix | Fixing something, but is not a user facing bug | |
| 106 | +| @performance | Improving performance | |
| 107 | +| @refactor | Moving or rewriting code, but does not change the behaviour | |
| 108 | +| @tweak | Changing some values or settings | |
| 109 | + |
| 110 | +Block comment sample |
| 111 | + |
| 112 | +```text |
| 113 | + ; Patch104p @bugfix commy2 13/09/2021 Add missing upgrade icon for Anthrax Beta. |
| 114 | +
|
| 115 | + UpgradeCameo1 = Upgrade_GLAAnthraxBeta |
| 116 | + UpgradeCameo2 = Upgrade_GLACamoNetting |
| 117 | +``` |
| 118 | + |
| 119 | +Inline comment sample |
| 120 | + |
| 121 | +```text |
| 122 | +ConditionState = REALLYDAMAGED RUBBLE NIGHT SNOW |
| 123 | + ... |
| 124 | + HideSubObject = Flag01A Flag01B Flag01C ; Patch104p @bugfix commy2 02/10/2022 Hides USA flag. |
| 125 | +End |
| 126 | +``` |
| 127 | + |
| 128 | +### 2. Pull Request Documentation |
| 129 | + |
| 130 | +The title of a new Pull Request is prefixed either with `Fix:` for being a fix or `Change:` for being a change that is |
| 131 | +not a fix, followed by a descriptive title of the change. Documentation and maintenance pulls are exempt of this rule. |
| 132 | + |
| 133 | +The text body begins with links to related issue report(s) and/or pull request(s) if applicable. |
| 134 | + |
| 135 | +To write a link, use the following format: |
| 136 | + |
| 137 | +```text |
| 138 | +* Relates to #555 |
| 139 | +* Follow up for #666 |
| 140 | +* Fixes #222 |
| 141 | +``` |
| 142 | + |
| 143 | +Some keywords are interpreted by GitHub. Read about |
| 144 | +it [here](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue). |
| 145 | + |
| 146 | +The text body continues with a description of the change in appropriate detail. This serves to educate reviewers and |
| 147 | +visitors to get a good understanding of the change without the need to study and understand the associated changed |
| 148 | +files. If the change is controversial or affects gameplay in a considerable way, then a rationale text needs to be |
| 149 | +appended. The rationale explains why the given change makes sense. |
| 150 | + |
| 151 | +### 3. Change Log Documentation |
| 152 | + |
| 153 | +Every change needs to be accompanied with a yaml definition file for the change log generation. It is supposed to |
| 154 | +contain concise information for the change. The yaml file needs to be created inside the `Patch104pZH/Design/Changes` |
| 155 | +folder or the appropriate subfolder. There is a template.yaml file that can be used as a base. The name of the file |
| 156 | +begins with the number of the Pull Request and a few words to summarize the essence of the change, seperated by |
| 157 | +underscores. It is possible that a new change extends an existing yaml file instead of creating a new one. In that case |
| 158 | +a txt file with the same naming rules can be created to refer to the actual yaml file. |
0 commit comments