Ren'Py Curious how this code could be improved.

[noping123]

Note: The following code is fully, 100% functional and does exactly what I want, and everything I want. It works. I'm just curious about ways it could be re-written to be "better". When I write code, I tend to hack things together until they work. I'll note - I fully understand HOW it works, and what it's doing, but it's usually a "learn as I go along and do it, and run into problems, figure out why I'm having them and how to fix it". Which basically means I know what I know, but I don't know what I don't know.

The code is for my phone/text messaging system. I'll post it in snippets, since it's spread out amongst multiple files all over the place.

First: The very basic python code behind most of it.

init python:
    import re

    class phoneMessage:
        def __init__(self,name,message):
            self.name = name
            self.message = message

    class phoneLog:
        def __init__(self):
            self.history = []

        def addmessage(self,name,message):
            self.history.append(phoneMessage(name,message))

        def delphone(self):
            self.history = []

Next, there's 3 seperate types of variable declarations. First assigns a variable, the next three are dictionaries referenced later on, and the last is just a list.

default Jasmine_msg = phoneLog()
default Jack_msg = phoneLog()
default Paisley_msg = phoneLog()
default Henry_msg = phoneLog()
default Samantha_msg = phoneLog()
default Marie_msg = phoneLog()
default chatstate = {'Jack':0,'Samantha':0,'Henry':0,'Jasmine':0,'Paisley':0,'Marie':0}
default newtext={'Jasmine':False,'Jack':False,'Paisley':False,'Henry':False,'Samantha':False,'Marie':False}
default msghistory ={'Jack':Jack_msg.history,'Samantha':Samantha_msg.history,'Henry':Henry_msg.history,'Jasmine':Jasmine_msg.history,'Paisley':Paisley_msg.history,'Marie':Marie_msg.history}
default contact_list=["Jack","Samantha","Henry","Marie"]

The next 3 are various screens that are used. The first is the contact list (shows all the contacts as buttons, which opens up a the second screen). The second screen is the actual text msgs, and the 3rd is the screen to select responses. Anything that is displayed as "new_idle", etc - it's just seperate buttons I have for a visual indication when there's new messages available.

contacts screen:

screen contacts():
    modal True
    add "phonecontacts" pos 800,150
    add "cdiv" pos 812,320
    text "Contacts" pos 905,265 style "contacts"
    imagebutton:
        idle "powerb"
        tooltip "Power"
        pos 971,809
        action Hide("contacts"),Show("phone")
    imagebutton:
        idle "back"
        tooltip "Back"
        pos 870,809
        action Hide("contacts"),Show("bigphone")

    vpgrid:
        mousewheel True
        draggable True
        cols 2
        spacing 20
        xpos 825
        ypos 330
        xsize 0.21
        ysize 0.42
        for i in contact_list:
            $tmp1 = i.lower() + "msg"
            $tmp2 = i + "chat" + str(chatstate[i])
            imagebutton:
                if newtext[i]:
                    idle "contact_" + i.lower() + "new_idle"
                    hover "contact_" + i.lower() + "new_hover"
                else:
                    idle "contact_" + i.lower() + "_idle"
                    hover "contact_" + i.lower() + "_hover"
                action Show('textmsgs',name=i,chatnum=tmp2), Hide("contacts")
            if i == "Jack":
                textbutton "Dad":
                    yalign 0.8
                    background None
                    action Show('textmsgs',name=i,chatnum=tmp2), Hide("contacts")
            elif i == "Samantha":
                textbutton "Mom" yalign 0.8 background None action Show('textmsgs',name=i,chatnum=tmp2), Hide("contacts")
            else:
                textbutton i yalign 0.8 background None action Show('textmsgs',name=i,chatnum=tmp2), Hide("contacts")

The 2nd screen is the actual messaging window that displays the texts: Notes - "extra" in the "not re.findall" section is to exclude certain images I want inline and not separate. images that are separated are simply a button that opens up a larger version of the image. I need background None there because default buttons (via gui styles) have a background. I have more buttons I *want* the background on than ones I don't, so it's easier to do it that way, than try and apply to all the ones I want it on. Finally for this, the on show action simply exists to prevent the message from endlessly looping - that's the same reason the "close this window" buttons are disabled while the label is active.

screen textmsgs(name,chatnum):
    $chatname = msghistory[name]
    on "show":
        if newtext[name]:
            action SetDict(chatstate,name,0),SetDict(newtext,name,False), Call(chatnum)
        else:
            action NullAction()
    default yadj = ui.adjustment()
    python:
        if yadj.value == yadj.range:
            yadj.value = float('inf')
    modal True
    add "phonechat" pos 800,150
    add "cdiv" pos 812,320
    add "contact_" + name.lower() + "_idle" pos 955,250
    if current_label == chatnum:
        imagebutton idle "powerb" pos 971,809 action NullAction()
        imagebutton idle "back" pos 870,809 action NullAction()
    elif freeroam:
        imagebutton idle "powerb" pos 971,809 tooltip "Power" action Hide("textmsgs"), Show("phone"), Jump(freeroamlabel)
        imagebutton idle "back" pos 870,809 tooltip "Back" action Hide("textmsgs"), Show("contacts"), Jump(freeroamlabel)
    else:
        imagebutton idle "powerb" pos 971,809 tooltip "Power" action Hide("textmsgs"), Show("phone")
        imagebutton idle "back" pos 870,809 tooltip "Back" action Hide("textmsgs"), Show("contacts")
    vpgrid yadjustment yadj:
        style_prefix "msgs"
        ypos 0.31
        xoffset 0
        yoffset 1
        ysize 0.43
        xsize 0.21
        cols 1
        spacing 0
        draggable True
        mousewheel True
        xfill True
        scrollbars "vertical"
        side_xalign 0.513
        yinitial 1.0
        vbox:
            spacing 5
            xalign 0.0 yalign 0.05
            xsize 260
            for i in chatname:
                if i.name == "MC":
                    hbox:
                        if re.findall(r'jpg|png', i.message, re.I) and not re.findall(r'extra', i.message, re.I):
                            xalign 0.75
                            frame:
                                background Frame(i.message,240,135)
                                button background None action Show('imagescrn4', img=i.message)
                        else:
                            xsize 360
                            frame:
                                background Frame("yc",10,10)
                                xmaximum 280
                                xalign 1.0
                                xoffset 0
                                text i.message style "textmsg"
                else:
                    hbox:
                        if re.findall(r'jpg|png', i.message, re.I) and not re.findall(r'extra', i.message, re.I):
                            xalign 0.0
                            frame:
                                xmaximum 250
                                ymaximum 250
                                xoffset 53
                                background Frame(i.message)
                                button background None action Show('imagescrn4', img=i.message)
                        elif re.findall(r'extra', i.message, re.I):
                            xalign 0.0
                            frame:
                                background Frame("tc",10,10)
                                xmaximum 250
                                xoffset 39
                                text i.message style "themmsg"
                        else:
                            xalign 0.0
                            frame:
                                background Frame("tc",10,10)
                                xmaximum 250
                                #xoffset 53
                                xanchor -0.17
                                text i.message style "themmsg"
    $tooltip = GetTooltip()

    if tooltip:
        nearrect:
            focus "tooltip"
            prefer_top False
            frame:
                background None
                xalign 0.5
                text tooltip size 24

The 3rd screen is just a response screen that gets called a lot:

screen msg_choice(choice1,choice2=False,choice3=False):
    vbox:
        xpos 1180
        yalign 0.7
        spacing 5
        if choice1:
            frame:
                xmaximum 320
                background Frame("yc",10,10)
                textbutton "[choice1]" text_style "textmsgchoice" background None action Return(choice1)
        if choice2:
            frame:
                xmaximum 320
                background Frame("yc",10,10)
                textbutton "[choice2]" text_style "textmsgchoice" background None action Return(choice2)
        if choice3:
            frame:
                xmaximum 320
                background Frame("yc",10,10)
                textbutton "[choice3]" text_style "textmsgchoice" background None action Return(choice3)

Finally, there's the "Actually using it" bit. First, when I want the msgs to be enabled I have something like this:

$chatstate['Jasmine'] = 1
$contact_list.append("Jasmine")
$newtext['Jasmine'] = True
$newtextmsg = True

(I could have defaulted "Jasmine" in the initial list in this instance, since it's the first time text msgs are being used in the game. I didn't because when I wrote that I was testing to make sure it worked properly, then just left it). When I want the msgs to be disabled, (Example, you passed that point in the game, and never checked it) I'll just reset the variables to 0 and False.

Finally, a snippet of the label where everything works. (I'll only give a partial bit, since it's fairly long):

label Jasminechat1:
    $Jasmine_msg.addmessage("Jasmine","images/chapter1/phone/phoneTP.png")
    call screen msg_choice("You actually got a picture? LOL!")
    if _return:
        $reply = _return
        $Jasmine_msg.addmessage("MC",reply)
    $Jasmine_msg.addmessage("Jasmine","I had to. I wonder if anyone ever told her.")
    call screen msg_choice("I saw her later and it was gone, so....")
    if _return:
        $reply = _return
        $Jasmine_msg.addmessage("MC",reply)
    $Jasmine_msg.addmessage("Jasmine","{image=images/phone/extra/laughemo.png}")
    $renpy.pause(1,modal=False)
    $Jasmine_msg.addmessage("Jasmine","Later {image=images/phone/extra/blood.png}")
    call screen msg_choice("Is... is that a threat?")
.

at the end of the label I do two things: I reset all the variables, so you can't loop through the msgs sequence again, and I run a small while loop to check if there are any more new texts. (The main phone screen not pictured here, has a notification for when you have new text msgs, that loop just checks if it should be turned off or kept on if you still have more you haven't checked).

Note: A big part of the reason for calling the "Response" screen even when there's only one available response, is simply to give the player a chance to read whatever is there, and CTC. I could easily just "add" the text and not bother with the screen call, but from a gameplay standpoint I didn't like that option as much.


All in all, the entire thing works exactly the way I want. I also tested it against everything I have planned for the future, and it worked correctly, so no issues later on either. Functionally, there is absolutely no reason to change a single thing. I'm just curious how this code could be re-written to be functionally identical, but "better" - because like I said to start, I don't know what I don't know.

One last note - if you look through the code and see things that don't look like they do anything at all, they probably don't. When I wrote this, it went through a few iterations, and I didn't fully cleanup the code once I settled on a working model. As I was pasting it I noticed at least one that I left behind and never got rid of, even though it does absolutely nothing - there's probably a couple of those I should clean up at some point.

 

[anne O'nymous]

I'm just curious about ways it could be re-written to be "better".

It works and you understand it. What can be "better" than this ? Why wanting it to be "better" ?

Oh, of course, it could possibly be more academical, it could possibly be speed, size, or code, optimized. And it could possibly follow a better logic, or a better paradigm. But in the end there's no interest in all this if it lead to hidden bugs and/or to a code that you would struggle to understand.

By example, some guy a bit too stuck on an academical approach could say that, since "phoneMessage" is only used by the "phoneLog" class, having it as independent class is stupid and useless. This would lead them to write it like that:

    class phoneLog:

        class phoneMessage:
            def __init__(self,name,message):
                self.name = name
                self.message = message

        def __init__(self):
            self.history = []

        def addmessage(self,name,message):
            self.history.append(self.phoneMessage(name,message))

        def delphone(self):
            self.history = []

So, yeah, of course now you clearly see that "phoneMessage" is a class fully dedicated to "phoneLog", and probably only used by it.
But there's no interest other than being what you've been taught. It's not faster, it's not shorter, it's not more beautiful, and it's not necessarily more understandable. It's just closer to the theory.

The only thing I have to reproach to your code is that class names should starts by an uppercase character. It's what permit to know that it's a class and not a variable. Therefore it should be "PhoneMessage" and "PhoneLog".
But this is also just a detail. It's just that it's a detail leading to a small optimization ; you differentiate better classes from variables.

Next, there's 3 seperate types of variable declarations. First assigns a variable, the next three are dictionaries referenced later on, and the last is just a list.

default Jasmine_msg = phoneLog()
[...]
default contact_list=["Jack","Samantha","Henry","Marie"]

Once again, this is correct and it works.

One could say that it's confusing, because there's information spread everywhere, but once again this is relative.
But, what is not relative, is the consequence of this spreading. It force you to have, somewhere else, codes that feel like they are fighting against something, in place where you could have code that would go with the flow.

Therefore, what would be "better" would be this:

init python:

    #  Back to the two separated classes, because academic code gives me pimples.
    #  With Ren'Py it would be implicit, but it's always a bit less confusing to explicitly
    # state the class inheritance.
    #  Note that I uppercased the name ; less confusing for my brain.
    class PhoneMessage( renpy.python.RevertableObject ):
        def __init__(self,name,message):
            self.name = name
            self.message = message

    class PhoneLog( renpy.python.RevertableObject ):

        def __init__(self, name ):
            self.history = []
            #  Those two are directly related to the discussion, they should be here, not spread in
            # weird places.
            self.state = 0
            self.waiting = False
            #  As for this one, it's a semi constant.
            self.name = name

         #  There's only two possible values for /name/. Either it's the name of the person the MC
        # talk too, or it's "MC". Therefore there's no need for a /name/ parameter, but for two
        # methods.
        def addmessage(self,message):
            self.history.append( PhoneMessage(self.name,message) )
            # Consequence of the related variables not being spread, you can automatically do this:
            self.waiting = True

        def addreply(self,message):
            self.history.append( PhoneMessage("MC",message) )

        def delphone(self):
            self.history = []

default messages = { "Jack": PhoneLog( "Jack" ), "Samantha": PhoneLog( "Samantha" ), [...] }
#  All the following become useless.
#default Jasmine_msg = phoneLog()
# [...]
#default Marie_msg = phoneLog()
#default chatstate = {'Jack':0,'Samantha':0,'Henry':0,'Jasmine':0,'Paisley':0,'Marie':0}
#default newtext={'Jasmine':False,'Jack':False,'Paisley':False,'Henry':False,'Samantha':False,'Marie':False}
#default msghistory ={'Jack':Jack_msg.history,'Samantha':Samantha_msg.history,'Henry':Henry_msg.history,'Jasmine':Jasmine_msg.history,'Paisley':Paisley_msg.history,'Marie':Marie_msg.history}
default contact_list=["Jack","Samantha","Henry","Marie"]

Of course, it's a bit more writing, to address Jack message history, you need to use messages["Jack"].history and not anymore msghistory["Jack"].

But in the same time, you can more easily switch from a value to another. Knowing if there's a new message from Jack is messages["Jack"].waiting, and getting his message history is messages["Jack"].history.
This being more intuitive and natural than the current newtext["Jack"] and msghistory["Jack"].

But the real point of this change is that it's way better from a code wise point of view...

contacts screen:

I pass on the lack of frame, vbox and other hbox, it's pure relativity here.
No, what matters is the consequences of your information being spread everywhere, and how better (without the quotation marks for once) it would be if they weren't.

screen contacts():
[...]
        for i in contact_list:
            $tmp1 = i.lower() + "msg"
            $tmp2 = i + "chat" + str(chatstate[i])
            imagebutton:
                if newtext[i]:
                    idle "contact_" + i.lower() + "new_idle"
                    hover "contact_" + i.lower() + "new_hover"
                else:
                    idle "contact_" + i.lower() + "_idle"
                    hover "contact_" + i.lower() + "_hover"
                action Show('textmsgs',name=i,chatnum=tmp2), Hide("contacts")
            if i == "Jack":
                textbutton "Dad":
                    yalign 0.8
                    background None
                    action Show('textmsgs',name=i,chatnum=tmp2), Hide("contacts")
            elif i == "Samantha":
                textbutton "Mom" yalign 0.8 background None action Show('textmsgs',name=i,chatnum=tmp2), Hide("contacts")
            else:
                textbutton i yalign 0.8 background None action Show('textmsgs',name=i,chatnum=tmp2), Hide("contacts")

It's precisely what I was saying above, your code is fighting against something ; something that is in fact itself.

There's a lot of computation that could be just made generic.
Here's what the screen looks like when you don't spread information everywhere, and use a class for what it is:

init python:

    [...]

    class PhoneLog( renpy.python.RevertableObject ):

        #  You can have parameters that have a default value when not specified. What come handy
        # here since some characters have a first name and an usual name.
        def __init__(self, name, nick=None ):
            self.history = []
            self.state = 0
            self.waiting = False
            self.name = name
            if nick is None:
                self.nick = name
            else:
                self.nick = nick

         [...]

        #  Classes aren't here to just store information, they are here to regroup all information
        # *and* computation related together.
        def idle( self ):
            txt = "contact_" + self.name.lower()
            if self.waiting:
                return txt + "new_idle"
            else:
                return txt + "_idle"

        def hover( self ):
            txt = "contact_" + self.name.lower()
            if self.waiting:
                return txt + "new_hover"
            else:
                return txt + "_hover"

default messages = { "Jack": PhoneLog( "Jack", nick="Dad" ), "Samantha": PhoneLog( "Samantha", nick="Mom" ), "Henry": PhoneLog( "Henry" ) [...] }
[...]

screen contacts():
[...]
        for i in contact_list:
            # Have you notice that you define this one, but never use it ?
            $tmp1 = i.lower() + "msg"
            # I'll let this one for now.
            $tmp2 = i + "chat" + str(chatstate[i])

            imagebutton:
                #  You don't need anymore to do the computation here, nor do you need to
                # test if there's a new message. The two methods will do all this for you.
                idle messages[i].idle()
                hover messages[i].hover()
                # I keep it like this for now.
                action Show('textmsgs',name=i,chatnum=temp2), Hide("contacts")

            #  Like for the /idle/ and /hover/ images, there's not need to test since using
            # an object offer you the possibility to automatize this.
            textbutton "messages[i].nick":
                yalign 0.8
                background None
                action Show('textmsgs',name=i,chatnum=tmp2), Hide("contacts")

The code is now two time shorter for the same result.

The 2nd screen is the actual messaging window that displays the texts: Notes - "extra" in the "not re.findall" section is to exclude certain images I want inline and not separate

[Emphasis is mine, and have a reason.]

init python:

    class PhoneMessage( renpy.python.RevertableObject ):
 
        def __init__(self,name,message):
            self.name = name
            self.message = message
            #  There's really no need to compute this every single time the screen is predicted and refreshed.
            # By the way, I hope they'll never have to discuss about image format...
            if re.findall(r'jpg|png', message, re.I) and not re.findall(r'extra', message, re.I):
                self.kind = "clickable"
            # ... and that no one will ever found that something is "extraordinary". Passing directly the kind as
            # flag would be way less "OMG there's a tons of bug possibilities here, it will blow soon or later".
            elsif re.findall(r'extra', message, re.I):
                self.kind = "static"
            else:
                self.kind = "text"

          self.sent = name == "MC"


    class PhoneLog( renpy.python.RevertableObject ):

         [...]

        def icon( self ):
            return "contact_" + self.name.lower() + "_idle"

        def label( self ):
            return "{}chat{}".format( self.name, self.state )


# Just a little change here...
screen contacts():
[...]
        for i in contact_list:
               [...]
               action Show('textmsgs', obj=messages[i] ), Hide("contacts")

            textbutton "messages[i].nick":
                yalign 0.8
                background None
                # The change is here.
                action Show('textmsgs', obj=messages[i] ), Hide("contacts")

# ... make a big change here.
screen textmsgs( obj ):
    #  There's no need to declare a variable that will be used only once.
    #  And anyway, /default/ would have been better here, there's no need to
    # redeclare it every single time the screen is refreshed.
    # $chatname = msghistory[name]

    # This way the /else/ is totally useless.
    if obj.waiting:
        on "show":
            action SetField( obj, "state", 0 ), SetField( obj, "waiting", False), Call( obj.label() )

    # Not really sure what you tried to achieve with this. Almost sure that there's a better way.
    default yadj = ui.adjustment()
    python:
        if yadj.value == yadj.range:
            yadj.value = float('inf')

    modal True
    add "phonechat" pos 800,150
    add "cdiv" pos 812,320
    # Once again it's what objects are for.
    add obj.icon() pos 955,250

    # Not really sure what you imply here, nor what the (previously named "chatnum") label effectively is.
    if current_label == obj.label():
        imagebutton idle "powerb" pos 971,809 action NullAction()
        imagebutton idle "back" pos 870,809 action NullAction()
    elif freeroam:
        #  Not really sure to like what seem implied by the show/jump couple. Especially with a called screen.
        # But anyway, Hide/Show couple is outdated.
        imagebutton idle "powerb" pos 971,809 tooltip "Power" action ToggleScreen("phone"), Jump(freeroamlabel)
        imagebutton idle "back" pos 870,809 tooltip "Back" action ToggleScreen("contacts"), Jump(freeroamlabel)
    else:
        imagebutton idle "powerb" pos 971,809 tooltip "Power" action ToggleScreen("phone")
        imagebutton idle "back" pos 870,809 tooltip "Back" action ToggleScreen("contacts")


    vpgrid yadjustment yadj:
        [...]
            for i in obj.history:
                #  Honestly, all this was a headache to decipher. And the fact that depending on the side
                # (sender/receiver) you're forced to adapt the offsets show that there's issues with the styles.

                hbox:
                    if i.sent:
                        xalign 0.75

                    if i.kind == "clickable":
                        imagebutton:
                            idle Frame( i.message, 240, 135 )
                            #  Could also be a method that directly build the style name accordingly to
                            # the kind and if it was sent or received.
                            if i.sent:
                                style "msgImgClick_sent"
                            else:
                                style "msgImgClick_received"
                            action Show('imagescrn4', img=i.message)

                    elsif i.kind == "static":
                        frame:
                            #  See above.
                            if i.sent:
                                style "msgImgStatic_sent"
                            else:
                                style "msgImgStatic_received"
                            #  This *is* your code... You're displaying what your explanation say as being 
                            # an image like if it was a text !!!
                            text i.message style "themmsg"
                            #  Pretty sure that it should be this instead
                            #add i.message

                    else:
                        frame
                            #  See above.
                            if i.sent:
                                style "msgImgText_sent"
                            else:
                                style "msgImgText_received"

                            text i.message style "themmsg"

    [...]

And of course, don't forget to define the styles.

Once again, classes are here to ease your work. They aren't just storage room for few data, but abstractions that hide the complexity of the computation behind explicit names that make the code easier to build and to understand.
In top of that, they permit to change your mind, or fix bugs, without the risk to miss "this almost never used part of the code where the computation also is". You change the code of the class method, and the change will automatically apply everywhere it's needed.

The 3rd screen is just a response screen that gets called a lot:

screen msg_choice(choice1,choice2=False,choice3=False):
    vbox:
        xpos 1180
        yalign 0.7
        spacing 5
        if choice1:
            frame:
                xmaximum 320
                background Frame("yc",10,10)
                textbutton "[choice1]" text_style "textmsgchoice" background None action Return(choice1)
        if choice2:
            frame:
                xmaximum 320
                background Frame("yc",10,10)
                textbutton "[choice2]" text_style "textmsgchoice" background None action Return(choice2)
        if choice3:
            frame:
                xmaximum 320
                background Frame("yc",10,10)
                textbutton "[choice3]" text_style "textmsgchoice" background None action Return(choice3)

What is nothing more than a complicated way to define a menu.

menu answerX( screen="msgChoice" ):
    "blabla":
        # What happen behind the [icode]Return( choice1 )
    "blabla":
        # What happen behind the [icode]Return( choice2 )
    "blabla":
        # What happen behind the [icode]Return( choice3 )

With of course "msgChoice" being the screen styled the way you want, would be way simpler than what you actually use.

Finally, a snippet of the label where everything works. (I'll only give a partial bit, since it's fairly long):

label Jasminechat1:
    $ messages[Jasmine].addmessage("images/chapter1/phone/phoneTP.png")
    # Players will hate this ! Do not present menu that have, and can only have, one single choice.
    menu( screen="msgChoice" ):
        "You actually got a picture? LOL!":
            $messages[Jasmine].addreply("You actually got a picture? LOL!")

    $messages[Jasmine].addmessage("I had to. I wonder if anyone ever told her.")
    menu( screen="msgChoice" ):
        "I saw her later and it was gone, so....":
            $messages[Jasmine].addreply("I saw her later and it was gone, so....")

    $messages[Jasmine].addmessage("Jasmine","{image=images/phone/extra/laughemo.png}")
    $renpy.pause(1,modal=False)
    $messages[Jasmine].addmessage("Jasmine","Later {image=images/phone/extra/blood.png}")
    menu( screen="msgChoice" ):
        "Is... is that a threat?":
            $messages[Jasmine].addreply("Is... is that a threat?")

All in all, the entire thing works exactly the way I want. I also tested it against everything I have planned for the future, and it worked correctly, so no issues later on either.

Yet you display static images through a text screen statement and with the same background than text messages, and your regEx will catch way more than you expect.

 

[noping123]

It works and you understand it. What can be "better" than this ? Why wanting it to be "better" ?

I don't want it to be "better" in that sense - I don't intend to redo or replace any of it -because as you said, it works, and I understand it. I'm just trying to use this as a sort of learning experience to inform stuff I'll build in the future.

Yet you display static images through a text screen statement and with the same background than text messages, and your regEx will catch way more than you expect.

The reason I do this is... the "extra" stuff, if you notice, is all in a folder called "extra". This is used exclusively for emoticons - hence why I want them displayed as text.

Originally I was trying to use an actual emoticon font, but renpy had issues displaying them properly in color, so I just opted to use images for them instead. The issue with that obviously, was the way I was displaying images separately as a button you could click to view a larger version of them. That's why I created the "extra" catch - and just put all the emoticons inside an "extra" folder.

So the way I have it set up right now - anything in the "extra" folder is treated as text(since it's all emoticons), and added along with the text. Anything not in that folder is treated as an actual image, and displayed as such.

# Not really sure what you imply here, nor what the (previously named "chatnum") label effectively is.

If you note the example I posted, it's label Jasminechat1:

Every label is like that. (So there's Jasminechat2, Jackchat1, and so on). The "chatnum" is just a variable for the current label to call - to initiate the correct message. The purpose of that line, is to "disable" the power buttons until the message sequence has run it's course.

# Not really sure what you tried to achieve with this. Almost sure that there's a better way.

Finally, this line is there to do the following: Autoscroll the message window to the bottom, AND allow proper scrolling through the message. This ensures that by default the most recent message (at the bottom) is always the one displayed by default, and players can properly scroll through the viewport to read through the entire message history.

There could be a better way? But this one was effective.


All in all, like I said, I'm not trying to "fix" or replace any of this code since it just works. But I do like seeing different ways of doing the exact same thing, so I can try to figure out how the "different" ways work - this way I can hopefully figure stuff out, and use that when I build new stuff in the future.

My code will never be "proper" or anything like that, and I'm not going to spend too much time trying to make it so, as long as I get it to work without bugs. But it is useful to try and figure out new or different ways to do things, so I can hopefully use some of that theory or knowledge when building new things.

 

[anne O'nymous]

The reason I do this is... the "extra" stuff, if you notice, is all in a folder called "extra". This is used exclusively for emoticons - hence why I want them displayed as text.

And you tested it, and text i.message displayed an image and not something like "images/extra/lol.png"... What a strange version of Ren'Py you have...

That's why I created the "extra" catch - and just put all the emoticons inside an "extra" folder.

What, as I said, will lead to issues when you'll have to use the word "extra" in one text message. This while a simple PhoneMessage( name, text, kind ) would avoid all possibility of confusion, and let you have your images wherever you want.

You must be registered to see the links

isn't just a mantra, it is THE mantra. The less there's useless computation, the less there's stupid bugs. And incidentally, the better it works.

Finally, this line is there to do the following: Autoscroll the message window to the bottom, AND allow proper scrolling through the message. This ensures that by default the most recent message (at the bottom) is always the one displayed by default, and players can properly scroll through the viewport to read through the entire message history.

So it's what I thought, there's a better way, the

You must be registered to see the links

. All your code is just a more complicated, and 5 years outdated, way to do this:

viewport:
    yinitial 1.0

All in all, like I said, I'm not trying to "fix" or replace any of this code since it just works.

It works in the range of the few possibilities you thought about and tested. Range that do not include the effective use of static images, and you didn't wondered what would happen if the letters "extra" appeared in a message.
Plus, I haven't changed it because I'm lazy, but using a regEx for something as simple as if "extra" in message: is a bit overkill.

My code will never be "proper" or anything like that, and I'm not going to spend too much time trying to make it so, as long as I get it to work without bugs.

The thing is that "proper" code is the only way to limits the risk of bugs. The less a code is proper, the more insidious bugs it hide, and the higher are the risks that it suddenly blow up in your face months, if not years, later.
It's why no one like to works with ten/twenty old code wrote by someone else. If they are so old, it mean that no one dare to touch them during all those years, and if no one dared to touch them, it's surely because they are dirty as shit. Those codes are landmines with a density so high that even an ant can't securely travel through it.



But well, what I'm saying is just me talking. You do like you want, personally I don't care. It's not my game, and it's not me that have to deal with this code, nor to remember for months/years what those strange computations can possibly means.

Yet, I'll quote

You must be registered to see the links

as conclusion: "Make it work, then make it beautiful, then if you really, really have to, make it fast. 90 percent of the time, if you make it beautiful, it will already be fast. So really, just make it beautiful!".

 

[noping123]

I'm not gonna disagree with anything you said, but I do have questions.

so -

And you tested it, and text i.message displayed an image and not something like "images/extra/lol.png"... What a strange version of Ren'Py you have...

Yes. 8.0.3 I believe. It works pretty much perfectly. Here's an example (cropped screenshot from the game):


The first one is the image, that's clickable, the second is the emoticon "image".



Now to the question:

What, as I said, will lead to issues when you'll have to use the word "extra" in one text message. This while a simple PhoneMessage( name, text, kind ) would avoid all possibility of confusion, and let you have your images wherever you want.

Explain how this would matter? I'm not seeing it. The way I understand it, the way it's coded, if an actual image appears in a text, it'll JUST be the image (never any words), so unless "extra" is in the imagename, it shouldn't be an issue.

If the word "extra" is in the actual text of the message, then it would be "caught", and treated as a text message.... which is exactly what I want to happen anyway.

I'm asking because it's possible I'm missing something, but I don't see how this would lead it to ever do anything I don't want it to do, unless I was dumb enough to use the word "extra" in an imagename.

 

[anne O'nymous]

Yes. 8.0.3 I believe. It works pretty much perfectly. Here's an example (cropped screenshot from the game):

Hmm...

[Long reflection, long rereading of your code]

Ok, missed the very last part (hidden in the use example), where in fact you don't provide an image but an image text tag...

In my defence, what is the interest to isolate emojis in a special directory, when you have an obvious "{image=" that is mandatory for the images to be displayed through a text screen statement ?
I guess the use of the most advanced way to examine a text, isn't totally for nothing in my confusion ; and also the reason why you needed a special directory since you know nothing about regEx syntax.

Explain how this would matter? I'm not seeing it. The way I understand it, the way it's coded, if an actual image appears in a text, it'll JUST be the image (never any words), so unless "extra" is in the imagename, it shouldn't be an issue.

It matter because you are still seeing only half of the issue... It works both side. The "extra" in text is not an issue, okay. But the "extra" in the image path or name, stay one.

 

[noping123]

So it's what I thought, there's a better way, the

You must be registered to see the links

. All your code is just a more complicated, and 5 years outdated, way to do this:

I wanted to resurrect this to see if you had any thoughts on a current problem I'm having.

So first point of note: I already use yinitial 1.0 in the code. (If you check the original textmsgs screen in my OP, it's there). yinitial only affects what it displays when the screen first opens however.

The purpose of the yadj code is this - while the screen is open, new text msgs are added which updates/refreshes the screen. With just yinitial, it will NOT autoscroll with the new updates - the yadj code exists specifically for that purpose.\


Now here comes the problem I've been running into:

That works perfectly, it does exactly what I want.

default yadj = ui.adjustment()
python:
if yadj.value == yadj.range:
yadj.value = float('inf')

then setting the viewport or vpgrid to "yadjustment yadj". If the current view is scrolled all the way down, it continues doing so. If you're not scrolled all the way down, it doesn't "jump" to the bottom. (Side note, I'd love to figure out how to get it to do that but so far I haven't).

That works. EXCEPT when there's a scene change. I've recently discovered that if you change scene while the screen is up, once the scene change happens, it STOPS auto-scrolling for some reason. If you manually scroll to the bottom, it continues to autoscroll again until another scene change.

Currently I have a really poor workaround in place, but I'd like to know either why it's happening, or how to fix it without a workaround.