-
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
Implementing names for characters #49
Conversation
Pull request works exactly as stated, names are generated for each character and displayed on the tooltip, very well done! Some ideas that I have for some of the logistic questions you proposed:
|
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 think you've had a little git problem here. This PR now also contains changes from your character overlay PR.
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.
As already noted, there are unrelated changes in this PR (character overlay). Please rebase or re-apply only the changes related to adding the names.
import org.terasology.registry.In; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Collection; | ||
import java.util.Random; | ||
|
||
import static org.terasology.namegenerator.creature.CreatureAssetTheme.COUNTRY; |
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.
As I've requested to change the name list prefix to "Old West" this (and probably other spots) need changes now... 🙄 I'm eager to merge Terasology/NameGenerator#23, though (after renames) 👍
import static org.terasology.namegenerator.creature.CreatureAssetTheme.COUNTRY; | |
import static org.terasology.namegenerator.creature.CreatureAssetTheme.OLD_WEST; |
@@ -124,6 +134,12 @@ private EntityRef spawnCitizen(EntityRef homeEntity) { | |||
|
|||
EntityRef entityRef = entityBuilder.build(); | |||
|
|||
if(entityRef.hasComponent(DisplayNameComponent.class)) { | |||
DisplayNameComponent displayNameComponent = entityRef.getComponent(DisplayNameComponent.class); | |||
CreatureNameProvider creatureNameProvider = new CreatureNameProvider(random.nextLong(), CreatureAssetTheme.COUNTRY); |
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.
CreatureNameProvider creatureNameProvider = new CreatureNameProvider(random.nextLong(), CreatureAssetTheme.COUNTRY); | |
CreatureNameProvider creatureNameProvider = new CreatureNameProvider(random.nextLong(), CreatureAssetTheme.OLD_WEST); |
SettlementRefComponent settlementRefComponent = entityRef.getComponent(SettlementRefComponent.class); | ||
trader.addComponent(settlementRefComponent); | ||
MarketComponent marketComponent = settlementRefComponent.settlement.getComponent(MarketComponent.class); | ||
if(entityRef.hasComponent(DisplayNameComponent.class)) { | ||
DisplayNameComponent displayNameComponent = entityRef.getComponent(DisplayNameComponent.class); | ||
CreatureNameProvider creatureNameProvider = new CreatureNameProvider(random.nextLong(), CreatureAssetTheme.COUNTRY); |
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.
CreatureNameProvider creatureNameProvider = new CreatureNameProvider(random.nextLong(), CreatureAssetTheme.COUNTRY); | |
CreatureNameProvider creatureNameProvider = new CreatureNameProvider(random.nextLong(), CreatureAssetTheme.OLD_WEST); |
if(entityRef.hasComponent(DisplayNameComponent.class)) { | ||
DisplayNameComponent displayNameComponent = entityRef.getComponent(DisplayNameComponent.class); | ||
CreatureNameProvider creatureNameProvider = new CreatureNameProvider(random.nextLong(), OLD_WEST); | ||
displayNameComponent.name = creatureNameProvider.generateName(); |
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.
Do you also need to save the component again?
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.
Yea, will add that. However, I feel like it doesn't really make a difference here.
if(entityRef.hasComponent(DisplayNameComponent.class)) { | ||
DisplayNameComponent displayNameComponent = entityRef.getComponent(DisplayNameComponent.class); | ||
CreatureNameProvider creatureNameProvider = new CreatureNameProvider(random.nextLong(), CreatureAssetTheme.OLD_WEST); | ||
displayNameComponent.name = "shopkeeper " + creatureNameProvider.generateName(); |
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.
Same here - save the component after modifying it?
Closed, this feature is implemented as part of #74. |
This is a basic implementation of names on characters.
There are still a bunch of logistic questions to be answered like whether we want to display the type of citizens too in the Tooltips. Would love to hear the community's opinions on this
Further, the names are being taken from a list of ELVEN names prefab in Name Generator. We could definitely add a prefab with Citizen names. Also the shopkeeper's name is rendered as a shopkeeper itself (again something I'd like to hear your opinions on).
And as always, a review is much appreciated :).
Added Terasology/NameGenerator#23 to Hopefully use Country names instead of ELVEN names, However sometimes, Names that do not belong to the list pop up in the citizen Tooltips.
Might need some testing and bug finding.