Issue(s) with mapping
26 views
Skip to first unread message
Simon Lundmark
Dec 23, 2024, 10:10:28 AM12/23/24
to LDMud Talk
Hi,
We are on driver 3.5.7 and migrated fairly recently (august/september).
We have noticed some issues with mappings:
Case one:
We have an alias mapping in our player object. It's really simple string->string mapping. I discovered that using:
string subst;
m_contains(&subst, aliases, verb);
Now means that subst is a reference to the actual string IN the mapping. So if I modify "subst", the value in the mapping also gets changed (in this case, a regexplode() followed by an assignment from an implode()-call)
Is this intended? I would expect that the passed argument gets filled with a copy of the element in the mapping for any basic types (int float string etc), while a complex type (array, mapping, object, struct) would be actually referenced "properly" into the mapping. This is definitely a change of behavior from before though, right? (or am I just going insane?)
Case two:
We have, in obscure cases, noticed that doing:
mapping m, a;
...
m = m + a;
Does not yield the expected result. And changing it to m += a; would instead yield the expected result.
And what is expected? All of the parts of a written into m, and any duplicates would be replaced in m by those in a. This was not the case (mostly things not overwritten), and while I was able to reproduce it in our mud I haven't been able to reproduce it in any of the mudlibs that comes with the driver. I have been chasing it off and on for a few months now and will continue to do so, but if there's anyone that has any good ideas for tracking/debugging/understanding the bug I'd gladly appreciate any ideas or suggestions.
Thanks,
Simon
Stephan Weinberger
Dec 23, 2024, 3:43:52 PM12/23/24
to ldmud...@googlegroups.com
Hello Simon,
On 23.12.24 16:10, Simon Lundmark wrote:
> Hi,
>
> We are on driver 3.5.7 and migrated fairly recently (august/september).
>
> We have noticed some issues with mappings:
>
> Case one:
> We have an alias mapping in our player object. It's really simple
> string->string mapping. I discovered that using:
> string subst;
> m_contains(&subst, aliases, verb);
>
> Now means that subst is a reference to the actual string IN the
> mapping. So if I modify "subst", the value in the mapping also gets
> changed (in this case, a regexplode() followed by an assignment from
> an implode()-call)
I cannot reproduce this with 3.6.7:
void test_m_contains()
{
int i;
string s;
mapping m = ([ "a": 1, "b": "foo" ]);
printf("%O\n", m);
m_contains(&i, m, "a");
printf("%O\n", i);
i = -1;
printf("%O\n", i);
printf("%O\n", m);
m_contains(&s, m, "b");
printf("%O\n", s);
s = "bar";
printf("%O\n", s);
printf("%O\n", m);
}
prints:
([ /* #1 */
"a": 1,
"b": "foo"
])
1
-1
([ /* #1 */
"a": 1,
"b": "foo"
])
"foo"
"bar"
([ /* #1 */
"a": 1,
"b": "foo"
])
as expected. Also the driver code in src/mapping.c does in fact copy the
value; and that code hasn't been changed in a long time.
Could it be that you stored references in the mapping in the first place?
If I do:
int i = 1;
string s = "foo";
mapping m = ([ "a": &i, "b": &s ]);
the code behaves exactly as you describe (and as expected in this case).
Sidenote: why not just do something like
verb = aliases[verb] || verb;
> Case two:
> We have, in obscure cases, noticed that doing:
> mapping m, a;
> ...
> m = m + a;
>
> Does not yield the expected result. And changing it to m += a; would
> instead yield the expected result.
Again, I cannot reproduce this with 3.6.7:
xc ([ "a": 1, "b": 2 ]) + ([ "a": -1, "c": 3 ])
CCALL: Result = ([ /* #1 */
CCALL: "c": 3,
CCALL: "a": -1,
CCALL: "b": 2
CCALL: ])
as expected and documented in doc/LPC/operators.
cya,
Invis
On 23.12.24 16:10, Simon Lundmark wrote:
> Hi,
>
> We are on driver 3.5.7 and migrated fairly recently (august/september).
>
> We have noticed some issues with mappings:
>
> Case one:
> We have an alias mapping in our player object. It's really simple
> string->string mapping. I discovered that using:
> string subst;
> m_contains(&subst, aliases, verb);
>
> Now means that subst is a reference to the actual string IN the
> mapping. So if I modify "subst", the value in the mapping also gets
> changed (in this case, a regexplode() followed by an assignment from
> an implode()-call)
void test_m_contains()
{
int i;
string s;
mapping m = ([ "a": 1, "b": "foo" ]);
printf("%O\n", m);
m_contains(&i, m, "a");
printf("%O\n", i);
i = -1;
printf("%O\n", i);
printf("%O\n", m);
m_contains(&s, m, "b");
printf("%O\n", s);
s = "bar";
printf("%O\n", s);
printf("%O\n", m);
}
prints:
([ /* #1 */
"a": 1,
"b": "foo"
])
1
-1
([ /* #1 */
"a": 1,
"b": "foo"
])
"foo"
"bar"
([ /* #1 */
"a": 1,
"b": "foo"
])
as expected. Also the driver code in src/mapping.c does in fact copy the
value; and that code hasn't been changed in a long time.
Could it be that you stored references in the mapping in the first place?
If I do:
int i = 1;
string s = "foo";
mapping m = ([ "a": &i, "b": &s ]);
the code behaves exactly as you describe (and as expected in this case).
Sidenote: why not just do something like
verb = aliases[verb] || verb;
> Case two:
> We have, in obscure cases, noticed that doing:
> mapping m, a;
> ...
> m = m + a;
>
> Does not yield the expected result. And changing it to m += a; would
> instead yield the expected result.
xc ([ "a": 1, "b": 2 ]) + ([ "a": -1, "c": 3 ])
CCALL: Result = ([ /* #1 */
CCALL: "c": 3,
CCALL: "a": -1,
CCALL: "b": 2
CCALL: ])
as expected and documented in doc/LPC/operators.
cya,
Invis
Simon Lundmark
Dec 24, 2024, 4:21:21 AM12/24/24
to ldmud...@googlegroups.com
I cannot reproduce this with 3.6.7:
...
as expected. Also the driver code in src/mapping.c does in fact copy the
value; and that code hasn't been changed in a long time.
Hi Stephen!
And thanks for your help!
Right, we're on 3.5.7 but it seems like the code for mappings hasn't changed between these versions. Thank you for validating that these cases are not easily reproducible.
If I in the middle of our function, I added the following code after the m_contains:
subst = sprintf("%s", substr);
Then calling the function would no longer modify the mapping (it's not a very complex function). The mapping is not storing any references to my knowledge. This is a mapping that's stored in the player object (which gets saved and loaded) and even if you add something to the mapping, relog (and thus load the saved mapping), it still happens. The insertion is very simple, it's basically my_mapping[string_1] = string_2; (but with other variable names).
I am unable to reproduce this outside of the player object. I've tried the exact same code in a different object and I'm unable to reproduce it there. The code even resides in an inheritable object, and I've tried inheriting it in another object and it simply does not happen there. None of the used efuns are overridden in the player object either.
Could it be that you stored references in the mapping in the first place?
Unfortunately, no. Unless loading and saving objects with mappings somehow does this?
Sidenote: why not just do something like
verb = aliases[verb] || verb;
Well, the code-base started back in 1992 I think and spans roughly 200 000 code files and m_contains is used in over a 1000 different places. Also we need to know whether or not the string exists in the mapping because then we do the part about regexplode etc.
> Case two:
Again, I cannot reproduce this with 3.6.7:
xc ([ "a": 1, "b": 2 ]) + ([ "a": -1, "c": 3 ])
CCALL: Result = ([ /* #1 */
CCALL: "c": 3,
CCALL: "a": -1,
CCALL: "b": 2
CCALL: ])
as expected and documented in doc/LPC/operators.
Yes, like stated I am unable to reproduce it "simply". It seems to be related to saved/load objects to be honest now that I think about it. In the case where this happens we have roughly 74 members of the mapping and I am unable to reproduce it in a simple way (I've tried to copy the mappings with exactly the same members) . Thanks for verifying these things, though my question was related to ideas for debugging these things since they are not easily reproducible.
Cheers,
Simon
Gnomi
Dec 24, 2024, 6:20:09 AM12/24/24
to ldmud...@googlegroups.com
Stephan Weinberger wrote:
> Could it be that you stored references in the mapping in the first place?
>
> If I do:
>
> int i = 1;
> string s = "foo";
> mapping m = ([ "a": &i, "b": &s ]);
>
> the code behaves exactly as you describe (and as expected in this case).
References currently work symmetrical, i.e. the value that is referencing
> Could it be that you stored references in the mapping in the first place?
>
> If I do:
>
> int i = 1;
> string s = "foo";
> mapping m = ([ "a": &i, "b": &s ]);
>
> the code behaves exactly as you describe (and as expected in this case).
and the value that is referenced are treated the same. This was mainly due
to technical reasons, as now both determine the lifetime of the value.
So the bevavior of the example above can also be caused by:
mapping m = (["A":"B"]);
string x = &(m["A"]);
string s;
m_contains(&s, m, "A");
s+="X"; // -> m = (["A": "BX"])
Instead of x = &(m["A"]) also walk_mapping() will create references, as it
needs to pass them to the function.
Sounds like I need to make them behave asymmetrical when reading.
With best regards and happy christmas,
Gnomi
Gnomi
Dec 24, 2024, 7:33:13 AM12/24/24
to ldmud...@googlegroups.com
On further thought, I disagree that this is expected behavior:
> Stephan Weinberger wrote:
> > Could it be that you stored references in the mapping in the first place?
> >
> > If I do:
> >
> > int i = 1;
> > string s = "foo";
> > mapping m = ([ "a": &i, "b": &s ]);
> >
> > the code behaves exactly as you describe (and as expected in this case).
References are designed to only be transferred with the '&' operator (or
with efuns that are documented to create references):
string a = "X";
string b = &a;
string c1 = b; // receives a copy
string c2 = &b; // receives the reference
c1 = "Y"; // doesn't change a and b
c2 = "Z"; // changes a and b as well.
Consequentially m_contains should only return references if the mapping was
passed via reference (which is not supported by m_contains right now). For
example foreach() has the option of either return a copy or reference,
depending on whether the expression is a reference or not.
So in the end m_contains should never return references even if the mapping
contains references, and therefore should be fixed.
With best regards,
Alex
> Stephan Weinberger wrote:
> > Could it be that you stored references in the mapping in the first place?
> >
> > If I do:
> >
> > int i = 1;
> > string s = "foo";
> > mapping m = ([ "a": &i, "b": &s ]);
> >
> > the code behaves exactly as you describe (and as expected in this case).
with efuns that are documented to create references):
string a = "X";
string b = &a;
string c1 = b; // receives a copy
string c2 = &b; // receives the reference
c1 = "Y"; // doesn't change a and b
c2 = "Z"; // changes a and b as well.
Consequentially m_contains should only return references if the mapping was
passed via reference (which is not supported by m_contains right now). For
example foreach() has the option of either return a copy or reference,
depending on whether the expression is a reference or not.
So in the end m_contains should never return references even if the mapping
contains references, and therefore should be fixed.
With best regards,
Alex
Simon Lundmark
Dec 25, 2024, 2:54:29 AM12/25/24
to ldmud...@googlegroups.com
Hi Gnomi!
On Tue, Dec 24, 2024 at 1:33 PM Gnomi <Gn...@unitopia.de> wrote:
References are designed to only be transferred with the '&' operator (or
with efuns that are documented to create references):
string a = "X";
string b = &a;
string c1 = b; // receives a copy
string c2 = &b; // receives the reference
c1 = "Y"; // doesn't change a and b
c2 = "Z"; // changes a and b as well.
Consequentially m_contains should only return references if the mapping was
passed via reference (which is not supported by m_contains right now). For
example foreach() has the option of either return a copy or reference,
depending on whether the expression is a reference or not.
So in the end m_contains should never return references even if the mapping
contains references, and therefore should be fixed.
With best regards,
Alex
Yes I agree, this makes sense.
I also have an update on how we somehow manage to get this behavior. Maybe it can put some clarity into the situation.
So I have an object, "aliases" which has a private mapping which I access through two functions. One for modifying (adding an alias) and one for fetching an alias with potential alias-arguments substituted.
Then I have another object "test", which inherits from "aliases".
Test loads data on init and saves data when I call the function "test2" on it.
test2 adds alias "test Test $*" through "addalias" (just aliases["test"] = "Test $*";) and then calls this function (with arguments "test" and ""):
nomask static string simon_test(string verb, string args) {
string subst;
if(!m_contains(&subst, aliases, verb)) return verb + args;
string* split = regexplode(subst, "\\$[0-9*][0-9\\-]*");
if(args == "") {
for(int i=sizeof(split)-2; i>=0; i-=2 ) split[i] = "";
subst = implode(split, "");
return subst[<1] == ' ' ? subst[0..<2] : subst;
}
return "BAD CODE PATH";
}
string subst;
if(!m_contains(&subst, aliases, verb)) return verb + args;
string* split = regexplode(subst, "\\$[0-9*][0-9\\-]*");
if(args == "") {
for(int i=sizeof(split)-2; i>=0; i-=2 ) split[i] = "";
subst = implode(split, "");
return subst[<1] == ' ' ? subst[0..<2] : subst;
}
return "BAD CODE PATH";
}
which is located in object "aliases".
That function then modifies the mapping to change "test" to be only "Test".
Now this does NOT happen if I have the code from aliases inside the test-object. It does NOT happen if I don't restore the "test" object before doing the operations.
I'm going to try to have a reproducible case that can be tested in the test mudlib later today.
Merry Christmas!
Thanks,
Simon
Simon Lundmark
Dec 25, 2024, 4:24:48 AM12/25/24
to ldmud...@googlegroups.com
Hi again,
I'm attaching two files, alias_bug.c and real_alias.c
If you boot the driver with the lp-245 mudlib and drop these two files into /obj
Then load and clone "alias_bug"
type 'test' (so that the object saves), then reboot mud or go south and relog so that the object is dropped. Log back in and clone "alias bug" again. You should see output like this:
clone alias_bug
Ok.
>
Ok.
>
test
You have 2 aliases defined as:
l look
test Test
MAPPING: ([ /* #1 */
"l": "look",
"test": "Test "
])
test = Test $* : Alias added.
You have 2 aliases defined as:
l look
test Test $*
MAPPING: ([ /* #1 */
"l": "look",
"test": "Test $*"
])
Subst: "Test"
You have 2 aliases defined as:
l look
test Test
MAPPING: ([ /* #1 */
"l": "look",
"test": "Test "
])
l look
test Test
MAPPING: ([ /* #1 */
"l": "look",
"test": "Test "
])
test = Test $* : Alias added.
You have 2 aliases defined as:
l look
test Test $*
MAPPING: ([ /* #1 */
"l": "look",
"test": "Test $*"
])
Subst: "Test"
You have 2 aliases defined as:
l look
test Test
MAPPING: ([ /* #1 */
"l": "look",
"test": "Test "
])
Where you can see that it prints the contents of the "aliases" mapping and in the call to simon_test which is done on line 20 in the alias_bug.c
It seems like the issue is triggered with the walk_mapping()-call on line 28 in real_alias.c
Which is really confusing. But if I remove that call, it doesn't happen.
I'll try to do some debugging in the next few days and see if I can step through the walk_mapping-call and find anything obvious.
Best,
Simon
Gnomi
Dec 25, 2024, 8:41:52 AM12/25/24
to ldmud...@googlegroups.com
Hi Simon,
I have a patch here: https://github.com/amotzkau/ldmud/commit/ac59764d8e9795ad63eabd6e1059edb73967014b
Could you verify that it fixes the problems with mappings?
With best regards,
Gnomi
I have a patch here: https://github.com/amotzkau/ldmud/commit/ac59764d8e9795ad63eabd6e1059edb73967014b
Could you verify that it fixes the problems with mappings?
With best regards,
Gnomi
Simon Lundmark
Dec 25, 2024, 1:14:33 PM12/25/24
to ldmud...@googlegroups.com
Hi Gnomi,
Wow that was fast!!
I have tested and verified that it solves the issue in the lpmud 2.4.5 mudlib on the 3.5.7 driver. I will ask it to be merged into our current live driver and validate that it fixes things there as well. I'm hoping that this could be solving some of the other issues we have been seeing related to mappings as well!
Thank you so much for looking into this.
Cheers,
Simon
--
You received this message because you are subscribed to the Google Groups "LDMud Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ldmud-talk+...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/ldmud-talk/Z2wLm3znjd9y3r66%40platinum.motzkau.
Simon Lundmark
Jan 1, 2025, 2:30:31 PMJan 1
to ldmud...@googlegroups.com
Hi Gnomi,
We have validated that this works in our mud as well.
However, it seems like m_delete no longer changes the reference passed into the function.
So:
mapping my_mapping = ([ "mytest": 7 ]);
m_delete(my_mapping, "mytest");
Gives a different result than what now needs to be:
mapping my_mapping = ([ "mytest": 7 ]);
my_mapping = m_delete(my_mapping, "mytest");
I also tried with
m_delete(&my_mapping, "mytest");
But that doesn't modify the passed in mapping either.
We use m_delete at atleast 950 different cases. Seems like we use m_delete without assignment in about 10% of those cases. So it's not a major thing to fix, but it would be nice to know the intended behavior here.
Thanks!
Simon
Alexander Motzkau
Jan 1, 2025, 3:16:08 PMJan 1
to ldmud...@googlegroups.com
Hi Simon,
m_delete() always returns the first argument (the argument will be just left on the stack, so it really will be the same). So the assignment of the result shouldn't change anything. Also Mappings are reference types: Any changes will be done on the same mapping data structure, no copy will be created. So there is no separate data structure without the changes.
Do you have working code that demonstrates the difference?
With best regards,
Gnomi
m_delete() always returns the first argument (the argument will be just left on the stack, so it really will be the same). So the assignment of the result shouldn't change anything. Also Mappings are reference types: Any changes will be done on the same mapping data structure, no copy will be created. So there is no separate data structure without the changes.
Do you have working code that demonstrates the difference?
With best regards,
Gnomi
Simon Lundmark
Jan 3, 2025, 6:10:46 AMJan 3
to ldmud...@googlegroups.com
Hi Gnomi,
My apologies.
It seems that we have overridden m_delete as a simul_efun where we deep-clone the map before calling the efun.
I'm sorry, I should have verified it in the base mudlib before sending it to you.
There is no problem here :)
Best regards,
Simon
--
You received this message because you are subscribed to the Google Groups "LDMud Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ldmud-talk+...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/ldmud-talk/CB291F1A-DADF-48F6-9BED-7BE53A4F1FFE%40web.de.
Zesstra
Jan 3, 2025, 6:15:32 AMJan 3
to ldmud...@googlegroups.com
Hi Simon,
Am 02.01.25 um 09:00 schrieb Simon Lundmark:
the behaviour of m_delete changed and to keep the old behaviour, many MUds
opted for such an sefun to keep the old behaviour.
Morgengrauen had it as well, but some 15 years back we "cleaned" the mess up
by having the original efun m_delete (standard case) and the sefun
m_copy_delete() for people preferring the old behaviour.
Best wishes,
Zesstra@MG
--
MorgenGrauen -
1 Welt, mehr als 200 Programmierer , mehr als 16000 Raeume,
viel mehr als 7000 unterschiedliche Figuren, 90 Quests, 13 Gilden,
ueber 5000 Waffen und Ruestungen, keine Umlaute und ein Haufen Verrueckter.
Existenz: mehr als 25 Jahre
http://mg.mud.de/
Am 02.01.25 um 09:00 schrieb Simon Lundmark:
> It seems that we have overridden m_delete as a simul_efun where we deep-clone
> the map before calling the efun.
I think, this was/is rather common in MUDs "with history". Very long time ago,
> the map before calling the efun.
the behaviour of m_delete changed and to keep the old behaviour, many MUds
opted for such an sefun to keep the old behaviour.
Morgengrauen had it as well, but some 15 years back we "cleaned" the mess up
by having the original efun m_delete (standard case) and the sefun
m_copy_delete() for people preferring the old behaviour.
Best wishes,
Zesstra@MG
--
MorgenGrauen -
1 Welt, mehr als 200 Programmierer , mehr als 16000 Raeume,
viel mehr als 7000 unterschiedliche Figuren, 90 Quests, 13 Gilden,
ueber 5000 Waffen und Ruestungen, keine Umlaute und ein Haufen Verrueckter.
Existenz: mehr als 25 Jahre
http://mg.mud.de/
Simon Lundmark
Jan 4, 2025, 6:07:15 AMJan 4
to ldmud...@googlegroups.com
Hi Zesstra,
Yeah we checked the history and it's from earlier than 1997. Funny enough, it's not used in the actual "copy" way anywhere (a = m_delete(b, c); where a and b is not the same or not expected to be the same). So we'll be cleaning that out and some other things that I found digging into our master object / lfuns.
It would be really helpful with a "best practices" for master/lfuns. Unfortunately the mudlibs that come with the driver seems quite old and outdated. I'll try to provide some PR's with some things that I think could be relevant usage for people.
Maybe there's some resource somewhere that I don't know of where people share these things (unless this mailing list is the place?)
Thanks,
Simon
--
You received this message because you are subscribed to the Google Groups "LDMud Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ldmud-talk+...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/ldmud-talk/dd45b889-2f32-4807-88cf-311f5715f296%40zesstra.de.
Reply all
Reply to author
Forward
0 new messages