I did a bit of investigative work into why menus aren't updating after our big merge before last sprint demo.
I think the issue is not that each wtext is not being updated, but they're always updating based off of the information given to the wtext text function when it is constructed at startup. So the codepath I'm referring to is:
FlightGui calls self._menu.set_caption(self) (by the way, in FlightGui.init, self._menu is wrongly annotated as a vpython.menu. Since self._menu is immediately set, it doesn't need a type annotation).
self._menu.set_caption calls functions like gui.get_reference(), gui.get_target(), etc. and gets the initial value of the reference and target and habitat.
set_caption iterates over a list of (sidebar gui element name: str, a function that calculates the text of the gui element: callable, help text: str, the new section flag: bool). This is the point where the new code changes break down. The original code referenced variables like self._reference and self._habitat in each of these help text lambda functions, but now that you've replaced all of these uses of self._reference with reference, each of these text functions will only ever use the copy of the entity that was assigned on menu.py:41, menu.py:42, and menu.py:43 the initial time that set_caption was called.
As a quick fix, for now we should just replace every line that says something like this:
In the future, we should make this scoping issue more explicit, likely by making a small subclass that encapsulates a reference to a FlightGui and owns a single wtext, and is responsible for updating that wtext.
As well, I think code like gui.concat_caption() which is implemented as a one-liner doesn't do much except add another layer to the call-stack. Since the data structure it's wrapping, gui._wtexts: List[vpython.wtext], is an internal implementation detail, I don't think there's any value by restricting the ways we can internally interact with this internal API. Defining functions like gui.concat_caption is useful if gui.concat_caption implemented an external API, but I think there may be a better way to express that separation internally.
Also, in Menu.__init__ we set self.ad = None but I don't see where that's used.
TL;DR I think the code line change I suggest above should solve it. Later (i.e. after the final demo) we may want to revisit how Menu is architected, but we don't have time right now.
Hey Kyoung,
I did a bit of investigative work into why menus aren't updating after our big merge before last sprint demo.
I think the issue is not that each wtext is not being updated, but they're always updating based off of the information given to the wtext text function when it is constructed at startup. So the codepath I'm referring to is:
self._referenceandself._habitatin each of these help text lambda functions, but now that you've replaced all of these uses ofself._referencewithreference, each of these text functions will only ever use the copy of the entity that was assigned on menu.py:41, menu.py:42, and menu.py:43 the initial time that set_caption was called.As a quick fix, for now we should just replace every line that says something like this:
with
and hopefully this should fix it.
In the future, we should make this scoping issue more explicit, likely by making a small subclass that encapsulates a reference to a FlightGui and owns a single wtext, and is responsible for updating that wtext.
As well, I think code like
gui.concat_caption()which is implemented as a one-liner doesn't do much except add another layer to the call-stack. Since the data structure it's wrapping,gui._wtexts: List[vpython.wtext], is an internal implementation detail, I don't think there's any value by restricting the ways we can internally interact with this internal API. Defining functions likegui.concat_captionis useful ifgui.concat_captionimplemented an external API, but I think there may be a better way to express that separation internally.Also, in
Menu.__init__we setself.ad = Nonebut I don't see where that's used.TL;DR I think the code line change I suggest above should solve it. Later (i.e. after the final demo) we may want to revisit how Menu is architected, but we don't have time right now.