-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Randomized physical NPC traits #74
Randomized physical NPC traits #74
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI check that's failing is because this branch is behind the current MR develop, specifically the BaseFlatWorldRasterizer
. Sync with that and all tests should pass.
src/main/java/org/terasology/metalrenegades/ai/system/CitizenPersonalitySystem.java
Show resolved
Hide resolved
src/main/java/org/terasology/metalrenegades/ai/system/CitizenPersonalitySystem.java
Outdated
Show resolved
Hide resolved
/* | ||
* Copyright 2020 MovingBlocks | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the shorter license header 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The license header has been changed 👍 The old license header still shows up for me when creating new files in IntelliJ, I have no idea why
/** | ||
* The +- range of possible model scaling differences. | ||
*/ | ||
public static final float SCALE_RANGE = 0.1f; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a deviation of 0.1 noticable in-game? Nice idea to actually differ them in size, though 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has changed now since character scale changes are done by multiplying model scale, but this offset range allows for scale differences from 0.15f-0.35f, pretty significant on the small scale of the gooey model
src/main/java/org/terasology/metalrenegades/ai/system/CitizenPersonalitySystem.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left one comment, otherwise looks good to me 👍 please change that little detail and merge yourself (that worked well before 🤓 )
public void onCitizenSpawn(CitizenSpawnedEvent citizenSpawnedEvent, EntityRef target) { | ||
NameTagComponent nameTagComponent = new NameTagComponent(); | ||
|
||
CreatureNameProvider nameProvider = new CreatureNameProvider(random.nextLong(), CreatureAssetTheme.OLD_WEST); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whould it be better to setup the nameProvider
once when initializing the system and then only use that generator for new names? generateName()
will generate a new name each time based on the seed of the name provider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that this is definitely better, the name provider is now initialized in the postBegin() stage. 👍
This pull request introduces some individual NPC characteristic randomization. When a character spawns, they are now given a name that displays over their head. As well, the characters now vary in size from small to large:
Two smaller-sized characters with names in the same house
The character names are generated using an old west preset of Terasology/NameGenerator.