-
-
Notifications
You must be signed in to change notification settings - Fork 0
Style Guide
Most functions that deal with components are retrieving access to them via getComponent in the beginning of the function definition:
const halfMyComponentValue = (entity: Entity) => {
const myComponent = getComponent(entity, MyComponent)
myComponent.value /= 2
}
const quarterMyComponentValue = (entity: Entity) => {
const myComponent = getComponent(entity, MyComponent)
myComponent.value /= 4
}However, most of these functions are being used in a context where the components have already been retrieved, or will need retrieved again in a following function, thus creating a lot of redundant getComponent calls:
const MySystem = (world) => {
const entities = query(world)
for (const entity of entities) {
const myComponent = getComponent(entity, MyComponent)
myComponent.value += 100
halfMyComponentValue(entity)
quarterMyComponentValue(entity)
}
}This potentially may have a significant impact on performance by inducing more lookups than are necessary. Instead, getComponent should be called once per entity iteration in a system, the reference of which can then be passed into respective function calls.
const halfMyComponentValue = (myComponent) => {
myComponent.value /= 2
}
const quarterMyComponentValue = (myComponent) => {
myComponent.value /= 4
}
const MySystem = (world) => {
const entities = query(world)
for (const entity of entities) {
const myComponent = getComponent(entity, MyComponent)
myComponent.value += 100
halfMyComponentValue(myComponent)
quarterMyComponentValue(myComponent)
}
}In this example, the number of lookups is reduced 3 * entities.length times. This also makes the functions more referentially transparent.
Most functions in the codebase do not exhibit referential transparency. This can considerably enhance the readability of code, making it easier to work with, especially those who are new to the codebase.
In all cases. it is best to write lots of small functions, rather than large functions or classes. This allows us to easily create test cases for each function. These functions can then be overridden and extended by projects to enable easy modifications to the base functionality. The functional paradigm also enables a data-oriented approach.
Functions should not rely on external state where possible, such as mutating variables outside of the function's scope (see Pure vs Impure Functions). The exception is when a function's purpose is to mutate state, such as a FLUX pattern receiver, a database mutation, or ECS interaction.
Functions should have clear and readable names, to aid ease of understanding. It is perfectly fine for these to be verbose, the module export can have shortened names if they are unreasonably long.
// CustomMathUtil.ts
// modules files must be named identitcally to their module, while lone function files must be named identically to the function they export
const addOne = (i: number) => {
return i++
}
// functions must be camelCase
const subtractOne = (i: number) => {
return i--
}
// modules must be CapitalCase
export const CustomMathUtil = {
addOne,
subOne
}When importing, it is important not to deconstruct these modules outside of the scope they are being used in, as the references will be kept, and if they have since been overridden, there will be a mismatch in functionality.
Bad
import { CustomMathUtil } from './CustomMathUtil'
const { addOne } = CustomMathUtil
function addTwo (number) {
return addOne(addOne(number))
}Good
import { CustomMathUtil } from './CustomMathUtil'
function addTwo (number) {
const { addOne } = CustomMathUtil
return addOne(addOne(number))
}To run all the unit & integration tests across the entire monorepo, use npm run test in the root folder. To run
Individual tests can be run by navigating to the package the specific test is running in (such as cd packages/engine) and running npm run test -- --grep describeHookLabel where 'describeHookLabel' is the label of a describe hook in a specific test file.
The goal with testing to is to provide a variety of input states to a function, and test that a variety of output states are as expected. We define 3 types of tests.
These test individual pieces of functionality, such as isolated functions. Examples of some well-written unit tests can be found here and here
All global state (such as Engine.userId) must be reset before and after as to not affect the tests. This will soon be refactored to not be an issue.
An example of the styling for writing tests can be found below. It is important to respect the gap lines as to ensure readable code.
import assert from 'assert'
import { CustomMathUtil } from './CustomMathUtil'
describe('CustomMathUtil', () => {
describe('addOne', () => {
// first test case
it('should add one', () => {
const result = CustomMathUtil.addOne(1)
assert.equal(result, 2)
})
// second test case
it('should return NaN to non-number', () => {
const result = CustomMathUtil.addOne(undefined!)
assert.equal(result, NaN)
})
})
})import { matches } from '@xrengine/engine/src/common/functions/MatchesUtils'
import { defineAction, defineState, dispatchAction, getState, useState } from '@xrengine/hyperflux'
export const ExampleState = defineState({
name: 'ExampleState',
initial: () => ({
counter: 0
})
})
export const ExampleServiceReceptor = (action) => {
getState(ExampleState).batch((s) => {
matches(action)
.when(ExampleAction.incrementCounter.matches, (action) => {
return s.merge({
counter: s.counter.value + 1
})
})
.when(ExampleAction.setCounter.matches, (action) => {
return s.merge({
counter: action.value
})
})
.when(ExampleAction.resetCounter.matches, (action) => {
return s.merge({
counter: 0
})
})
})
}
export const accessExampleState = () => getState(ExampleState)
export const useExampleState = () => useState(accessExampleState())
export const ExampleService = {
incrementCounterPeriodically: async (period = 1000) => {
setInterval(() => {
dispatchAction(ExampleAction.incrementCounter())
}, period)
}
}
export class ExampleAction {
static incrementCounter = defineAction({
type: 'example.INCREMENT_COUNTER'
})
static setCounter = defineAction({
type: 'example.SET_COUNTER',
value: matches.number
})
static resetCounter = defineAction({
type: 'example.RESET_COUNTER'
})
}With all that said, here is an example of a big need we currently have, which is breaking all our big convoluted functions into smaller ones, that can easily be tested.
export class User extends Service {
app: Application
docs: any
constructor(options: Partial<SequelizeServiceOptions>, app: Application) {
super(options)
this.app = app
}
async find(params: Params): Promise<any> {
if (!params.query) params.query = {}
const { action, $skip, $limit, search, ...query } = params.query!
const skip = $skip ? $skip : 0
const limit = $limit ? $limit : 10
delete query.search
if (action === 'friends') {
delete params.query.action
const loggedInUser = extractLoggedInUserFromParams(params)
const userResult = await (this.app.service('user') as any).Model.findAndCountAll({
offset: skip,
limit: limit,
order: [['name', 'ASC']],
include: [
{
model: (this.app.service('user-relationship') as any).Model,
where: {
relatedUserId: loggedInUser.id,
userRelationshipType: 'friend'
}
}
]
})
params.query.id = {
$in: userResult.rows.map((user) => user.id)
}
return super.find(params)
} else if (action === 'layer-users') {
delete params.query.action
const loggedInUser = extractLoggedInUserFromParams(params)
params.query.instanceId = params.query.instanceId || loggedInUser.instanceId || 'intentionalBadId'
return super.find(params)
}
// ...
}
}becomes...
export const userServiceFindActionFriends = async (user: User, params: Params) => {
const skip = $skip ? $skip : 0
const limit = $limit ? $limit : 10
delete params.query.action
const loggedInUser = extractLoggedInUserFromParams(params)
const userResult = await (user.app.service('user') as any).Model.findAndCountAll({
offset: skip,
limit: limit,
order: [['name', 'ASC']],
include: [
{
model: (user.app.service('user-relationship') as any).Model,
where: {
relatedUserId: loggedInUser.id,
userRelationshipType: 'friend'
}
}
]
})
params.query.id = {
$in: userResult.rows.map((user) => user.id)
}
return Service.prototype.find.call(user, params)
}
export const userServiceFindActionLayerUsers = async (user: User, params: Params) => {
delete params.query.action
const loggedInUser = extractLoggedInUserFromParams(params)
params.query.instanceId = params.query.instanceId || loggedInUser.instanceId || 'intentionalBadId'
return Service.prototype.find.call(user, params)
}
export class User extends Service {
app: Application
docs: any
constructor(options: Partial<SequelizeServiceOptions>, app: Application) {
super(options)
this.app = app
}
async find(params: Params): Promise<any> {
if (!params.query) params.query = {}
const { action, $skip, $limit, search, ...query } = params.query!
delete query.search
if (action === 'friends') {
return userServiceFindActionFriends(this, params)
} else if (action === 'layer-users') {
return userServiceFindActionLayerUsers(this, params)
}
// ...
}
}