About the r8145/r8146 Commit
Moderator: Moderators
About the r8145/r8146 Commit
To fix the sub for the 0291/"message_string" packet is a good idea.
But I think it will be better that using "parseROLUT" rather than "parseArrayFile".
That is it should include the Msg ID in the msgstringtable.txt file.
And make the fomate like "nnnn#xxxxxxx#".
The "nnnnn" is Msg ID, and the "xxxxxxx" is the message string.
But I think it will be better that using "parseROLUT" rather than "parseArrayFile".
That is it should include the Msg ID in the msgstringtable.txt file.
And make the fomate like "nnnn#xxxxxxx#".
The "nnnnn" is Msg ID, and the "xxxxxxx" is the message string.
-
- Developers
- Posts: 1798
- Joined: 05 Dec 2008, 05:42
- Noob?: Yes
Re: About the r8145/r8146 Commit
http://openkore.svn.sourceforge.net/vie ... threv=8145
Any change in $args would mess with additional packet handlers, and here it incremented twice which probably neither correct nor needed. I'd make such @msgTable so message ID in packet can be used as is (for example, shift @msgTable in onLoaded option of addTableFile).
Any change in $args would mess with additional packet handlers, and here it incremented twice which probably neither correct nor needed. I'd make such @msgTable so message ID in packet can be used as is (for example, shift @msgTable in onLoaded option of addTableFile).
-
- Moderators
- Posts: 208
- Joined: 01 Feb 2010, 17:37
- Noob?: No
- Location: Brazil - MG
- Contact:
-
- Developers
- Posts: 1798
- Joined: 05 Dec 2008, 05:42
- Noob?: Yes
Re: About the r8145/r8146 Commit
Yes.
You can just inline that sub like this:
Also, calling T like this is useless:
Just skip it in this case.
T and TF should only be used when there's any real "text" in source code (not in data) to translate, and all variable data should be passed in additional arguments to TF. Refer to gettext docs if you need details on that.
You can just inline that sub like this:
Code: Select all
onLoaded => sub { shift @msgTable }
Code: Select all
T($anyVariable."\n")
T and TF should only be used when there's any real "text" in source code (not in data) to translate, and all variable data should be passed in additional arguments to TF. Refer to gettext docs if you need details on that.
Re: About the r8145/r8146 Commit
I means use Hash is rather than Array to keep the message strings from msgstringtable.txt file.
Using Array, the order and number of lines must be keep well, because the index of the Array is the Msg ID.
Using Hash, the order and number of lines could be changed, because them are in key to value pairs.
Using Array, the order and number of lines must be keep well, because the index of the Array is the Msg ID.
Using Hash, the order and number of lines could be changed, because them are in key to value pairs.
Re: About the r8145/r8146 Commit
And I think the "sub message_string" in ServerType0.pm should be changed to:
Code: Select all
sub message_string {
my ($self, $args) = @_;
if (@msgTable[$args->{msg_id}++]) { # show message from msgstringtable
warning T(@msgTable[$args->{msg_id}++]."\n");
if ($args->{msg_id} >= 0x04F2 && $args->{msg_id} <= 0x04F5) {
$self->mercenary_off ();
}
} else {
if ($args->{msg_id} == 0x04F2) {
message T("Mercenary soldier's duty hour is over.\n"), "info";
$self->mercenary_off ();
} elsif ($args->{msg_id} == 0x04F3) {
message T("Your mercenary soldier has been killed.\n"), "info";
$self->mercenary_off ();
} elsif ($args->{msg_id} == 0x04F4) {
message T("Your mercenary soldier has been fired.\n"), "info";
$self->mercenary_off ();
} elsif ($args->{msg_id} == 0x04F5) {
message T("Your mercenary soldier has ran away.\n"), "info";
$self->mercenary_off ();
} elsif ($args->{msg_id} == 0x054D) {
message T("View player equip request denied.\n"), "info";
} elsif ($args->{msg_id} == 0x06AF) {
message T("You need to be at least base level 10 to send private messages.\n"), "info";
} else {
warning TF("msg_id: %s gave unknown results in: %s\n", $args->{msg_id}, $self->{packet_list}{$args->{switch}}->[0]);
}
}
}
-
- Developers
- Posts: 1798
- Joined: 05 Dec 2008, 05:42
- Noob?: Yes
Re: About the r8145/r8146 Commit
Well, original msgstringtable is already like this, and there's no need to change it.sofax222 wrote:Using Array, the order and number of lines must be keep well, because the index of the Array is the Msg ID.
Well, we already have our own strings here, it won't help with anything. Actually, message_string uses not so many strings from msgstringtable, they could just all be put here so we know what kind of events message_string may indicate. As a debug measure usage of msgstringtable is good, but that's all.sofax222 wrote:And I think the "sub message_string" in ServerType0.pm should be changed to:
-
- Moderators
- Posts: 208
- Joined: 01 Feb 2010, 17:37
- Noob?: No
- Location: Brazil - MG
- Contact:
Re: About the r8145/r8146 Commit
Sorry for this warning T(, it was just a distractionEternalHarvest wrote:Yes.
You can just inline that sub like this:Also, calling T like this is useless:Code: Select all
onLoaded => sub { shift @msgTable }
Just skip it in this case.Code: Select all
T($anyVariable."\n")
T and TF should only be used when there's any real "text" in source code (not in data) to translate, and all variable data should be passed in additional arguments to TF. Refer to gettext docs if you need details on that.
Thats fine, but if you change anything in msgstringtable you lose the benefits of the file being ready to use. Using an array you just need to extract the file from the GRF and insert it in the proper server's table. With a hash we would just change it from the actual format to anything like this:sofax222 wrote:I means use Hash is rather than Array to keep the message strings from msgstringtable.txt file.
Using Array, the order and number of lines must be keep well, because the index of the Array is the Msg ID.
Using Hash, the order and number of lines could be changed, because them are in key to value pairs.
(currentline +1)#(string)
I think that's pretty redundant. Unless gravity changes msgstringtable, but then we can think of something about it.
-
- Super Moderators
- Posts: 801
- Joined: 06 May 2008, 12:47
- Noob?: No
Re: About the r8145/r8146 Commit
In the following line, T has no use:
(Only staticly known strings can be translated by gettext.)
Also, why is it a warning? I'm convinced it should be debug.
This is what I and Eternal agreed upon on irc:
Btw, with commit r8149 ST0 & kRO diverged again, this packethander can easily be refactored to receive.pm, so the ST's converge there.
Also I don't like that there are now 2 codepaths with duplicated mercenary logic there (msgstringtable.txt is loaded AND the ID can be found OR not), those code paths are prone to diverging and can thus cause bugs that are hard to find.
Code: Select all
warning T(@msgTable[$args->{msg_id}++]."\n");
Also, why is it a warning? I'm convinced it should be debug.
This is what I and Eternal agreed upon on irc:
Meaning, we should probably not rely on it all too much.EternalHarvest wrote:As a debug measure usage of msgstringtable is good, but that's all.
Btw, with commit r8149 ST0 & kRO diverged again, this packethander can easily be refactored to receive.pm, so the ST's converge there.
Also I don't like that there are now 2 codepaths with duplicated mercenary logic there (msgstringtable.txt is loaded AND the ID can be found OR not), those code paths are prone to diverging and can thus cause bugs that are hard to find.
Ideally, we want to move all environment and AI stuff out of the servertypes.
We want to be able to attach not 1 but many packet-callback-handlers to a given received packet. (now we only have the message_string callbackhandler)
Brace yourselves, PSEUDOCODE...
$message_string_packet->addEventListener($callbackhandler-that-checks-for-mercenary-logic-stuff-and-takes-action);
$message_string_packet->addEventListener($callbackhandler-that-displays-a-message);
$message_string_packet->addEventListener($callbackhandler-that-checks-other-stuff-and-takes-action);
One ST0 to rule them all? One PE viewer to find them!
One ST_kRO to bring them all and in the darkness bind them...
Mount Doom awaits us, fellowship of OpenKore!
One ST_kRO to bring them all and in the darkness bind them...
Mount Doom awaits us, fellowship of OpenKore!