merge dccwidget and dc6widget

[gucio321]

currently, there are two animation widgets: dccwidget and dc6widget. Both has generally the same features. IMO they should be merged into a one widget (e.g. animationwidget)

[gucio321]

also, I think that it should be done before a serious llook at #298

[GibranHL0]

Hey 👋, I'm new to the project and I'd like to contribute.
Could you assign me this issue?

[GibranHL0]

Hey @gucio321,

I've been working on an approach to this issue and I believe that the merging would be difficult because the DCC and DC6 widgets access different properties of their respective structures.

Their structures on the OpenDiablo2 look like this

type DC6 struct {
	Version            int32
	Flags              uint32
	Encoding           uint32
	Termination        []byte // 4 bytes
	Directions         uint32
	FramesPerDirection uint32
	FramePointers      []uint32    // size is Directions*FramesPerDirection
	Frames             []*DC6Frame // size is Directions*FramesPerDirection
}

type DCC struct {
	Signature          int
	Version            int
	NumberOfDirections int
	FramesPerDirection int
	Directions         []*DCCDirection
	directionOffsets   []int
	fileData           []byte
}

What I've been thinking is to avoid code repetition, as some tasks between the creation of both widgets are the same. So, I think it would be a good solution to add a new layer of abstraction in the new animationwidget package, widget, and widgetstate, that would hold DCC and DC6 structures. The widget, Dc6Widget, and DccWidget structures would look like this:

type Widget struct {
	id            string
	palette       *[256]d2interface.Color
	textureLoader hscommon.TextureLoader
}

type Dc6Widget struct {
	widget *Widget
	dc6    *d2dc6.DC6
}

type DccWidget struct {
	widget *Widget
	dcc    *d2dcc.DCC
}

Comparing to the current structures:

// DC6 widget
type widget struct {
	id            string
	dc6           *d2dc6.DC6
	textureLoader hscommon.TextureLoader
	palette       *[256]d2interface.Color
}

// DCC Widget
type widget struct {
	id            string
	dcc           *d2dcc.DCC
	palette       *[256]d2interface.Color
	textureLoader hscommon.TextureLoader
}

Do you think that adding a new abstraction would help instead of merging both widgets?

[gravestench]

@GibranHL0 This is a good scenario for using an interface.

IIRC, giu.Widget is an interface that only needs a Build() method:

type Widget interface {
    Build()
}

There are a few things in common between the DCC and DC6 image files:

• They both have directions, which are ordered sequences of images
• They both are paletted images, so require a palette to be drawn.

with this information, we can define some interfaces to describe these commonalities:

type hasDirection interface {
	Direction() int
	SetDirection(int)
}

type hasFrames interface {
	Frame() int
	SetFrame(int)
}

type hasPalette interface {
	Palette() color.Palette
	SetPalette(color.Palette) error
}

Altogether, an AnimationWidget might look something like this:

type AnimationWidget interface {
	giu.Widget
	hasPalette
	hasDirection
	hasFrames
}

Exactly how you go about implementing is up to you, but I would try to separate the common stuff like this:

type state struct {
	currentDirection, currentFrame int
	palette color.Palette
}

type Widget struct {
	id            string
	*state
}

func (w *Widget) Build() {
	/*
	could do common stuff here,
	called from dcc or dc6 widget Build method
	*/
}

func (w *Widget) Palette() color.Palette {
	return w.state.palette
}

func (w *Widget) SetPalette(p color.Palette) error {
	if len(p) != colorsPerPalette {
		return fmt.Errorf("expected %v colors, but palette has %v colors", colorsPerPalette, len(p))
	}

	w.state.palette = p

	return nil
}

func (w *Widget) Frame() int {
	return w.state.currentFrame
}

func (w *Widget) SetFrame(n int) {
	w.state.currentFrame = n
}

func (w *Widget) Direction() int {
	return w.state.currentDirection
}

func (w *Widget) SetDirection(n int) {
	w.state.currentDirection = n
}

Then, the concrete animation widget implementations could look like this:

type Dc6Widget struct {
	*Widget
	dc6    *d2dc6.DC6
}

func (w *Dc6Widget) Build() {
	w.Widget.Build() // call the generic one

	// do DC6 specific stuff
}

type DccWidget struct {
	*Widget
	dcc    *d2dcc.DCC
}

func (w *DccWidget) Build() {
	w.Widget.Build() // call the generic one

	// do DCC specific stuff
}

EDIT:
It's also important to note that because *Widget is embedded inside of the DccWidget and Dc6Widget, the methods of Widget are also embedded! This means that Dc6Widget and DccWidget would implement the AnimationWidget interface. :)

You could check for this by adding the following code at the top of your file after the imports:

var _ AnimationWidget = &DccWidget{}
var _ AnimationWidget = &Dc6Widget{}

Those two lines will prevent the code from compiling if DccWidget or Dc6Widget do not implement AnimationWidget

[GibranHL0]

@gravestench Thanks for the time for writing such a detailed explanation. It was very helpful!

I'm currently implementing the interfaces to form the animation widget.

I hope to have some news about the PR soon!

Again, thanks!