Skip to content

Commit 1b6b22e

Browse files
committed
Merge branch 'master' into pzgulyas/gltf2
2 parents 362e4e7 + 7915033 commit 1b6b22e

File tree

186 files changed

+43511
-9596
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

186 files changed

+43511
-9596
lines changed

Docs/Code Guidelines.md

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# Open Rails Code Guidelines
2+
3+
## Logging
4+
5+
- Use the most appropriate [logging severity](#logging-severity)
6+
- Use the most appropriate logging method:
7+
- When parsing files, use [file format logging methods](#file-format-logging-methods)
8+
- Otherwise, use [general logging methods](#general-logging-methods)
9+
- Use [simple past tense](https://en.wikipedia.org/wiki/Simple_past) with the verb at the front; e.g. "Ignored missing texture file" instead of "Missing texture file was ignored".
10+
- Do not make reference to the user or Open Rails itself
11+
- Include the most relevant source file and line number at the end of the message (_file format logging methods_ do this for you)
12+
13+
### Logging severity
14+
15+
- Error: Fatal problem where continuing is not possible (only used in very limited places)
16+
- Warning: Resolved problem or manageable bad data (e.g. data that is missing/duplicate/unknown/unexpected)
17+
- Ignored missing ...
18+
- Ignored duplicate ...
19+
- Skipped unknown ...
20+
- Expected ...; got ...
21+
- Information: No problem but is notable (e.g used default for important but commonly missing data)
22+
- Used default for ...
23+
24+
### File format logging methods
25+
26+
When parsing files, use the specific logger methods below to automatically include accurate source information (file name, line number):
27+
28+
- When using `JsonReader`, use `JsonReader.TraceWarning` and `JsonReader.TraceInformation`
29+
- When using `SBR.Open`, use `SBR.TraceWarning` and `SBR.TraceInformation`
30+
- When using `STFReader`, use `STFException.TraceWarning` and `STFException.TraceInformation` (static methods)
31+
32+
### General logging methods
33+
34+
- For application start-up (adjacent to existing code), use [Console.Write](https://learn.microsoft.com/en-gb/dotnet/api/system.console.write) and [Console.WriteLine](https://learn.microsoft.com/en-gb/dotnet/api/system.console.writeline)
35+
- For exceptions when loading a file, use [Trace.WriteLine](https://learn.microsoft.com/en-gb/dotnet/api/system.diagnostics.trace.writeline) as follows:
36+
```csharp
37+
try
38+
{
39+
// Something that might break
40+
}
41+
catch (Exception error)
42+
{
43+
Trace.WriteLine(new FileLoadException(fileName, error));
44+
}
45+
```
46+
- For exceptions in other cases, use [Trace.WriteLine](https://learn.microsoft.com/en-gb/dotnet/api/system.diagnostics.trace.writeline) as follows:
47+
```csharp
48+
try
49+
{
50+
// Something that might break
51+
}
52+
catch (Exception error)
53+
{
54+
Trace.WriteLine(error);
55+
}
56+
```
57+
- Otherwise, use [Trace.TraceError](https://learn.microsoft.com/en-gb/dotnet/api/system.diagnostics.trace.traceerror), [Trace.TraceWarning](https://learn.microsoft.com/en-gb/dotnet/api/system.diagnostics.trace.tracewarning), [Trace.TraceInformation](https://learn.microsoft.com/en-gb/dotnet/api/system.diagnostics.trace.traceinformation), for example:
58+
```csharp
59+
Trace.TraceError("Player locomotive {1} cannot be loaded in {0}", conFileName, wagonFileName);
60+
Trace.TraceWarning("Skipped unknown texture addressing mode {1} in shape {0}", shapeFileName, addressingMode);
61+
Trace.TraceInformation("Ignored missing animation data in shape {0}", shapeFileName);
62+
```

Docs/Contributing.md

Lines changed: 57 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
# Contributing to Open Rails
22

3-
This document will introduce you to a number of ways you can contribute to Open Rails, and how we expect the process to go - both from your side and our side.
3+
This document will introduce you to a number of ways you can contribute to Open Rails and how we expect the process to go - from your side and our side.
44

55
## Discussion
66

7-
Please see the [Community](http://openrails.org/share/community/) page on our website for details of the forums where Open Rails discussion happens.
7+
If you'd like to discuss anything about Open Rails, please visit [our forums on Elvas Tower](https://www.elvastower.com/forums/index.php?/forum/190-open-rails-simulator-project/).
88

99
## Reporting a bug
1010

@@ -22,7 +22,7 @@ In most cases, you can get started immediately with making the changes and creat
2222

2323
**Note:** You must fork the Open Rails repository before you start working on it. We do not allow you to push branches to the official repository.
2424

25-
**Note:** You should do your work on separate branches; they must be created from the "master" branch and pull requests must merge back into the "master" branch, unless we direct you otherwise.
25+
**Note:** You should do your work on separate branches; they must be created from the default branch and pull requests must merge back into the default branch, unless we direct you otherwise.
2626

2727
### Documentation and translations
2828

@@ -38,13 +38,13 @@ There are no additional requirements for the pull request.
3838

3939
### Refactoring process
4040

41-
If you'd like to refactor the existing code you can get started immediately, but please have a look at our [architecture requirements](#architecture-requirements). We welcome architectural discussions on our [forum](http://www.elvastower.com/forums/index.php?/forum/256-developing-features/).
41+
If you'd like to refactor the existing code you can get started immediately, but please have a look at our [architecture requirements](#architecture-requirements). We welcome architectural discussions in [our Developing Features forum on Elvas Tower](https://www.elvastower.com/forums/index.php?/forum/256-developing-features/).
4242

4343
There are no additional requirements for the pull request.
4444

4545
### Bug process
4646

47-
If you'd like to fix a bug, you can get started immediately. If the fix turns out to be very small, you do not even need a bug report. Otherwise, you will need to make sure it has been reported on [our bug tracker on Launchpad](https://bugs.launchpad.net/or). If it has not, you can report the bug *and* fix it!
47+
If you'd like to fix a bug you can get started immediately. If the fix turns out to be very small, you do not even need a bug report. Otherwise, you will need to make sure it has been reported on [our bug tracker on Launchpad](https://bugs.launchpad.net/or). If it has not, you can report the bug _and_ fix it!
4848

4949
There are no additional requirements for _creating_ the pull request.
5050

@@ -54,7 +54,7 @@ These things must be done in the required order:
5454

5555
### Feature process
5656

57-
If you'd like to add a feature, you can get started immediately. However, we would prefer you to to do some things first. These will ensure that people are aware you are working on a particular feature and give the community some time to resolve any potential issues.
57+
If you'd like to add a feature you can get started immediately. However, we would prefer you to to do some things first. These will ensure that people are aware you are working on a particular feature and give the community some time to resolve any potential issues.
5858

5959
The following diagram shows the required order (solid lines) and recommended order (dashed lines):
6060

@@ -74,7 +74,7 @@ flowchart
7474
All new features must result in the following three things existing:
7575

7676
1. A road-map card in [Trello](https://trello.com/b/DS2h3Pxc/open-rails-roadmap)
77-
2. A forum discussion in [Elvas Tower](http://www.elvastower.com/forums/index.php?/forum/299-open-rails-development-testing-and-support/) more than one week old with all issues resolved
77+
2. A forum discussion in [Elvas Tower](https://www.elvastower.com/forums/index.php?/forum/256-developing-features/) more than one week old with all issues resolved
7878
3. A pull request
7979

8080
These things must be done in the required order:
@@ -97,23 +97,27 @@ Our recommended order is:
9797

9898
If you do not know what to work on, you can find bugs and features we are interested in fixed/adding here:
9999

100-
* [Confirmed bugs](https://bugs.launchpad.net/or/+bugs?orderby=-importance&field.status%3Alist=TRIAGED)
101-
* [Accepted feature requests (anything in an N.M or N.x list)](https://trello.com/b/DS2h3Pxc/open-rails-roadmap)
100+
- [Confirmed bugs](https://bugs.launchpad.net/or/+bugs?orderby=-importance&field.status%3Alist=TRIAGED)
101+
- [Accepted feature requests (anything in an N.M or N.x list)](https://trello.com/b/DS2h3Pxc/open-rails-roadmap)
102102

103103
If multiple things are interesting to you, we would prefer that you choose the item with the highest priority to us - a higher importance or heat in Launchpad bugs and lowest version number in Trello cards.
104104

105-
If you're unsure what you could contribute to in the code, and nothing looks interesting in the _confirmed bugs_ and _accepted feature requests_, please get in touch on the [Elvas Tower forums](http://www.elvastower.com/forums/index.php?/forum/299-open-rails-development-testing-and-support/), giving us some idea of your experience and interests, and we'll do our best to find something for you.
105+
If you're unsure what you could contribute to in the code, and nothing looks interesting in the _confirmed bugs_ and _accepted feature requests_, please get in touch using [our forums on Elvas Tower](https://www.elvastower.com/forums/index.php?/forum/190-open-rails-simulator-project/), giving us some idea of your experience and interests, and we'll do our best to find something for you.
106+
107+
## Requirements for changes
106108

107109
### General requirements
108110

109111
All of the main Open Rails code is C# and your contribution is expected to also be in C#. We're currently using [version 7.3 of C#](https://docs.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-7-3), so please take advantage of these features.
110112

111-
Code is expected to follow the [Framework Design Guidelines](https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/) throughout, especially the [Naming Guidelines](https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/naming-guidelines), with few exceptions:
113+
Code is expected to follow the [Microsoft .NET Framework Design Guidelines](https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/) throughout, especially the [Microsoft .NET Naming Guidelines](https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/naming-guidelines), with few exceptions:
114+
115+
- Structures, fields, and enums defining file format components may be named exactly as in the file format
116+
- Public and protected fields are allowed, although care must be taken with public fields
112117

113-
* Structures, fields, and enums defining file format components may be named exactly as in the file format
114-
* Public and protected fields are allowed, although care must be taken with public fields
118+
Code style (placement of braces, etc.) is expected to follow the default Visual Studio rules; the [Microsoft .NET C# Coding Conventions](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/inside-a-program/coding-conventions) provides a good basis for many aspects of this.
115119

116-
Code style (placement of braces, etc.) is expected to follow the default Visual Studio rules; the [C# Coding Conventions](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/inside-a-program/coding-conventions) provides a good basis for many aspects of this.
120+
Code should be consistent with the [Open Rails Code Guidelines](Code%20Guidelines.md).
117121

118122
### Architecture requirements
119123

@@ -133,46 +137,39 @@ All fixed values used in formula must be placed in a constant, with a name and u
133137

134138
Open Rails is a multi-threaded application, which presents some additional complexity. There are four key threads to be aware of:
135139

136-
* Loader
137-
* Updater
138-
* Render
139-
* Sound
140+
- Loader
141+
- Updater
142+
- Render
143+
- Sound
140144

141-
Data that is operated only on one thread for its lifetime does not need special attention. However, any data that is operated on by multiple threads - even if only one thread is writing - needs special care and attention.
145+
Data that is operated on by only one thread for its lifetime does not need additional care and attention. However, any data that is operated on by multiple threads - even if only one thread is making changes - needs additional care and attention.
142146

143147
For each object stored in a field or property that is accessed from multiple threads, the root, you must:
144148

145-
* Never modify the contents of the objects within the root (such as adding or removing items from a List<T>)
146-
* Always copy the root object into a local variable before doing anything else with it
147-
* Update the root object by (as above) copying into a local, cloning/making a new version from the old version, and finally storing into the root
148-
* If multiple threads can update the root, the final store into root must be done using an interlocked compare-and-exchange with a loop in case of failure
149-
150-
If you are in any doubt about the use of data by multiple threads, or your implementation of the above rules, please ask in the [Elvas Tower](http://www.elvastower.com/) forums.
151-
152-
### Getting your code accepted
153-
154-
Your code should be fixing exactly one bug or adding a single new feature; mixing multiple bug fixes or new features makes it harder to review your changes and risks them not being accepted.
155-
156-
### Different versions of code
149+
- Never modify the contents of the objects within the root (such as adding or removing items from a List<T>)
150+
- Always copy the root object into a local variable before doing anything else with it
151+
- Update the root object by (as above) copying into a local, cloning/making a new version from the old version, and finally storing into the root
152+
- If multiple threads can update the root, the final store into root must be done using an interlocked compare-and-exchange with a loop in case of failure
157153

158-
When your pull request is draft or ready for review, it will not be included in any version of Open Rails unless:
154+
If you are in any doubt about the use of data by multiple threads, or your implementation of the above rules, please ask for help in [our forums on Elvas Tower](https://www.elvastower.com/forums/index.php?/forum/190-open-rails-simulator-project/).
159155

160-
* You are a member of the core team
161-
* A member of the core team adds a particular label
156+
## Submitting changes
162157

163-
If your pull request satisfies the above criteria, it will be automatically included in the Unstable Version (unless there are merge conflicts).
158+
**Note:** Your code should be fixing exactly one bug or adding a single new feature; mixing multiple bug fixes or new features makes it harder to review your changes and risks them not being accepted.
164159

165-
After your pull request is merged, it will be included in the Testing Version and Unstable Version.
160+
When you're done writing code, you should make a pull request on GitHub from your fork's branch back to the official repository's default branch. The title and description of the requests should concisely indicate what bug or feature you've implemented and you will need to include links to whichever of the following are appropriate:
166161

167-
When we start preparing for a new Stable Version, all code in the Testing Version is used, but no further changes are included during the preparation time (typically 1 month).
162+
- Bug report
163+
- Road-map card
164+
- Blueprint
168165

169-
### Submitting your code
166+
### When changes are published
170167

171-
When you're done writing code, you should make a pull request on GitHub from your fork's branch back to the official repository's "master" branch. The title and description of the requests should concisely indicate what bug or feature you've implemented and you will need to include links to whichever of the following are appropriate:
168+
Your changes will not be included in any version of Open Rails immediately:
172169

173-
* Bug report
174-
* Road-map card
175-
* Blueprint
170+
- To be included in the _Unstable Version_, your pull request needs a particular label, which we encourage [our developer team](https://launchpad.net/~ordevs/+members) to add.
171+
- To be included in the _Testing Version_, your pull request needs to be [reviewed, approved, and merged](#how-to-review-pull-requests).
172+
- To be included in the _Stable Version_, your pull request needs to be merged before the branch point (typically a month before release).
176173

177174
## How bugs and features are accepted
178175

@@ -192,24 +189,24 @@ We require that a [forum thread is created](http://www.elvastower.com/forums/ind
192189

193190
A member of [our management team](https://launchpad.net/~orsupervisors/+members) will read the request and follow the forum discussion being had by the community, and approve its direction if appropriate.
194191

195-
## Reviewing pull requests
192+
## How to review pull requests
196193

197194
If you are reviewing someone else's code for Open Rails, you will need to ensure that they have met the above "Making changes" guidelines as best as possible. This will necessitate, at minimum:
198195

199-
* Check for linked bug report or feature request
200-
* Check bug report is triaged, and feature request is approved
201-
* For a bug report, it should have status "Triaged"
202-
* For a road-map card, it should be in an N.M or N.x list
203-
* For a blueprint, it should have direction "Approved"
204-
* Read through all of the changes to the code
205-
* Check that all new code follows the requirements:
206-
* General (including naming)
207-
* Architecture
208-
* Physics
209-
* Multi-threading
210-
* Be sure that all of the changes are necessary
211-
* Be sure that no changes are missing
212-
* Be on the lookout for data being access across threads
196+
- Check for linked bug report or feature request
197+
- Check bug report is triaged, and feature request is approved
198+
- For a bug report, it should have status "Triaged"
199+
- For a road-map card, it should be in an N.M or N.x list
200+
- For a blueprint, it should have direction "Approved"
201+
- Read through all of the changes to the code
202+
- Check that all new code follows the requirements:
203+
- [General](#general-requirements) (including naming)
204+
- [Architecture](#architecture-requirements)
205+
- [Physics](#physics-requirements)
206+
- [Multi-threading](#multi-threading-requirements)
207+
- Be sure that all of the changes are necessary
208+
- Be sure that no changes are missing
209+
- Be on the lookout for data being access across threads
213210

214211
### Leeway when reviewing
215212

@@ -219,8 +216,8 @@ You should take extra care when reviewing first-time and new contributors, to en
219216

220217
For all contributions that deviate from the guidelines, there are a few approaches you can take:
221218

222-
* Politely and constructively suggest changes on the pull request (if possible, include the desired code)
223-
* Make the changes yourself (GitHub provides instructions to push changes to other people's pull requests)
224-
* Accept the code as-is, leaving a note for how to improve for the next pull request
219+
- Politely and constructively suggest changes on the pull request (if possible, include the desired code)
220+
- Make the changes yourself (GitHub provides instructions to push changes to other people's pull requests)
221+
- Accept the code as-is, leaving a note for how to improve for the next pull request
225222

226223
It is expected that most contributors will quickly correct their code based on feedback, either in the same pull request or subsequent ones, depending on the path taken above. However, if a contributor continues to not meet the same part of the guidelines, you are free to become more strict with them - it's still helpful to suggest the corrected code, but do not feel obliged to spend time helping the same person with the same part of the guidelines repeatedly.

0 commit comments

Comments
 (0)