Skip to content
This repository has been archived by the owner on Aug 20, 2022. It is now read-only.

Dictionary system | WIP #2

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

LuaFan2
Copy link

@LuaFan2 LuaFan2 commented Aug 16, 2020

closes #1
Work In Progress

@benank benank requested review from benank and rdev34 August 16, 2020 20:47
@benank
Copy link
Member

benank commented Aug 16, 2020

Should this be a singleton?

EDIT: Added comment in review.

self:SetValue(lang, phrases)
end

function Dictionary:SetLanguage(lang)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't expect the current language to be changed during runtime, lets move this to the global config so it's easily configured.
Please make a class like this in napi/shared to store a config value for current language:

NapiConfig = class()

function NapiConfig:__init()
   self.Language = LanguageEnum.English
end

NapiConfig = NapiConfig()

Now we can access it with NapiConfig.Language

@@ -0,0 +1,51 @@
Dictionary = class(ValueStorage)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need to inherit ValueStorage for Dictionary since all we're doing with it is storing some data in this class. You could just have a self.current_language variable in this class which would be more readable and simple. We use ValueStorage for in-game entities for easily setting and getting data on them.

end

return phrase
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to initialize the singleton at the end of the file, with

Dictionary = Dictionary()

(applies to any/all singletons)

@@ -0,0 +1,51 @@
Dictionary = class(ValueStorage)
Copy link
Collaborator

@rdev34 rdev34 Aug 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should refactor all this code like this:

  1. LanguageEnum class to define languages
  2. PhraseEnum class to define phrases
  3. Phrases class as the interface for any gamemode logic

I see it working like this:

LanguageEnum = immediate_class(Enum)

function LanguageEnum:__init()
  self:EnumInit()
  
  self.English = 1
  self.Spanish = 2

  self:SetDescription(self.English, "English")
  self:SetDescription(self.Spanish, "Spanish")
end

LanguageEnum = LanguageEnum()
PhraseEnum = immediate_class(Enum)

function PhraseEnum:__init()
  self:EnumInit()

  self.WelcomeText = 1
end

PhraseEnum = PhraseEnum()
Phrases = class()

function Phrases:__init()
  self.translations = {
    [PhraseEnum.WelcomeText] = {
        [LanguageEnum.English] = "Welcome to the server!",
        [LanguageEnum.Spanish] = "Bienvenidos!"
    }
    -- etc
  }
end

function Phrases:GetPhrase(phrase_enum)
  local language = NapiConfig.Language
  local translation = self.translations[phrase_enum]
  assert(phrase_enum ~= nil, "Phrase does not have any translation")
  assert(translation[language] ~= nil, "Phrase does not have a translation in " .. LanguageEnum:GetDescription(language))
  return translation[language]
end

Phrases = Phrases()

Now we can do local text = Phrases:GetPhrase(PhraseEnum.WelcomeText)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't better will be to create language enums and phrases in different files (not in class)?(one for each language in special directory)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will at least make it easier to add phrases in different languages to the repository.

Copy link
Collaborator

@rdev34 rdev34 Aug 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - it would be good to put all the phrases for each language in its own file. You could create a directory in napi/shared and come up with a way to do this. Then, you could use that data to set-up the self.translations variable in Phrases. I would probably create a class for each language and map a string to a PhraseEnum.

Also, we are not creating the enums in the Phrases class. We are just creating a table so we can look up a translation for a Phrase in a given Language. The enums are good practice and should be kept for readability and for the sake of providing a unique id to each Phrase and Language.

Copy link
Author

@LuaFan2 LuaFan2 Aug 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also such system where all is Enum it's a wildness for me if we want to make it simple to use by develoler in his own gamemode. If languages are initialized in only one place, it creates certain limitations.
Not better to make it to work so?:
-- russian.lua
Russian = Language("Russian")

Russian:Phrase("greetings", "Добро пожаловать!")
Russian:Phrase("kicked", "Вы были кикнуты с сервера")

-- config.lua
NapiConfig:SetLanguage("Russian")
or
NapiConfig.language = "Russian"
-- script.lua
print(Language:GetPhrase("kicked"))

it seems to be the most simplified and dev-friendly variant. If SetLanguage not exists then we take English Language, if phrase not exists then we took it in English.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Add a language module
4 participants