Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Images doesn't return all the image in the embed. #1508

Open
4 tasks done
AlphaGaming7780 opened this issue Jul 30, 2023 · 12 comments
Open
4 tasks done

[BUG] Images doesn't return all the image in the embed. #1508

AlphaGaming7780 opened this issue Jul 30, 2023 · 12 comments
Assignees
Labels
bug Something isn't working on hold This issue/PR is on hold pending higher priority issues/PRs

Comments

@AlphaGaming7780
Copy link

Library Version

5.8.0

Describe the Bug

I set 2 images in an embed using this code :

newEmbed = initialPost.embeds[0]
newEmbed.images = [
	EmbedAttachment(
		url=newEmbed.images[0].url
	),
	EmbedAttachment(
		url=modal_ctx.responses["ArtSubmitionPreviewImagelink"]
	)
]

Once this code is run, the embed look like this and it has 2 images:
image

After I try to take the URL of the second image using this code line: newEmbed.images[1].url
result :

    newEmbed.images[1].url
    ~~~~~~~~~~~~~~~^^^
IndexError: list index out of range

cause when I print the images property using this line : print(newEmbed.images) I have : [EmbedAttachment()]

I also try to edit the first image using this code :

newEmbed.images[0] = EmbedAttachment(
	url=modal_ctx.responses["ArtSubmitionBigImageLink"]
)

Result : the image get updated, but it removes the second image.
image

Steps to Reproduce

It's explained above.

Expected Results

When using the images property, it should return all the EmbedAttachment().

Minimal Reproducible Code

No response

Traceback

No response

Checklist

  • I have searched the open issues for duplicates.
  • I have shown the entire traceback, if possible.
  • I have removed my token from display, if visible.
  • I have attempted to debug this myself, and I believe this issue is with the library

Additional Information

No response

@AstreaTSS
Copy link
Member

Admittedly, images is just an abstraction over some logic that's really only meant to be used for sending embeds, not receiving them. That being said, it probably is possible to make this work "as expected".

@AlphaGaming7780
Copy link
Author

it probably is possible to make this work "as expected".

Would be nice, caused I can't find any other way to get the link of the second image or a way to edit a specific EmbedAttachment.

@i0bs i0bs self-assigned this Jul 30, 2023
@i0bs
Copy link
Contributor

i0bs commented Jul 30, 2023

Have you tried using .set_images() instead? We're using the builder paradigm there and gives you a new version of Embed: Self to use. It seems your issue is coming from the way you're writing the images, not accessing

@i0bs
Copy link
Contributor

i0bs commented Jul 30, 2023

Admittedly, images is just an abstraction over some logic that's really only meant to be used for sending embeds, not receiving them. That being said, it probably is possible to make this work "as expected".

There's three solutions I can currently think of for resolving:

  1. 💥 Make .images a property method and add a setter method.
  2. Modify attrs decorator to use slots=False (True by default if unspecified)
  3. Modify/declare via. .set_images(), and chain with .images[idx] (let idx represent literal index)

@i0bs
Copy link
Contributor

i0bs commented Jul 30, 2023

Have you tried using .set_images() instead?

This approach seems to work fine:

>>> embed = interactions.Embed(title="pseudoembed")
>>>
>>> # .set_images() -> Embed[T@Self]
>>> embed = embed.set_images([
>>>     "https://www.randomkittengenerator.com/cats/156755.2.jpg",
>>>     "https://www.randomkittengenerator.com/cats/44190.jpg",
>>> ])
>>>
>>> print(embed.images[1].url)

https://www.randomkittengenerator.com/cats/44190.jpg

Keeping issue open for other comments. If none pending – labelling as wontfix and closing within 24hr for SLAs.

@AstreaTSS
Copy link
Member

AstreaTSS commented Jul 30, 2023

After a bit of discussion, we've determined that this is indeed a problem when getting the embed from the sent message - that being said, the images can still be found in the list of embeds (each image should have its own embed due to how the multi-image concept works), and then you can modify those embeds to your will. Thus, this isn't a bug per se, just unexpected behavior.

This isn't possible to make it work as expected without a breaking change, according to some contributors, so unfortunately we can't do much else.

@AlphaGaming7780
Copy link
Author

If I use set_images() the second image would be removed.

And I don't think there is a way, to get the URL of the second image.

I know the "multiple images" in an embed is a "bug", but would be nice even if it's a bug to create a way to retrieve all images, since we don't know when it's going to be fixed and it's maybe going to be fixed by removing the "url" necessary.

I actually manage by editing the lib code to retrieve these data (even if I completely don't understand how it works) whiteout recoding the whole file.

    @property
    def images(self) -> Optional[list[EmbedAttachment]]:
        """
        The images of the embed.
        """
        
        return self.images if self.images else None

    @images.setter
    def images(self, value: Optional[list[EmbedAttachment]]) -> "Embed":
        """Set the images of the embed."""
        self.images = [] if value is None else value
        return self

Please don't judge me, I know this code is crap, but like how I say, I don't understand how it works.

But after all this part of code once added in the file makes me able to retrieve all the images data from Embed, the only strange thing is now, I need to use this: .images[1]["url"], instead of .images[1].url, but at least it's working fine.

Note : I want to apologize for my English, I'm not an English speaker and I'm on my phone, so it's worse.

@AstreaTSS
Copy link
Member

I'm pretty sure we got a misunderstanding going on, but point is, we'll be looking at it. The fix isn't super simple (the example you provided would be a breaking change), but we might be able to come up with something.

@i0bs
Copy link
Contributor

i0bs commented Jul 30, 2023

The example you've provided is the 1st out of 3 possible resolutions I've listed above in the comment history. This would be a breaking change for us.

[...], the only strange thing is now, I need to use this: .images[1]["url"]

This is because there's an issue we have with deserialising the data. As you see in our Embed declaration, we're using converters to parse dict schema data into the object – this is why url is treated as a key mapping, instead of the object referenced itself.

@i0bs
Copy link
Contributor

i0bs commented Jul 31, 2023

If I use set_images() the second image would be removed.
And I don't think there is a way, to get the URL of the second image.

Are you saying it would be removed because the second image lacks a URL, or because the second parameter is ignored? The internals for that method are in vogue doing the same as directly assigning to .images, it just returns back the object you mutated, hence embed = embed.set_images().

Additionally, using .set_images() should allow you to treat indices of .images as referenceable objects instead of a mapping.

@AlphaGaming7780
Copy link
Author

Are you saying it would be removed because the second image lacks a URL, or because the second parameter is ignored?
First option, I don't have the link any more.

And I found another interesting thing, if you run this code on a message with an embed that have multiple images, the “new” embed wouldn't have all the images, only the first one.

	initial_post = THE_MESSAGE
	await initial_post.edit(embed=initial_post.embeds[0])

@LordOfPolls
Copy link
Contributor

So the neat thing about muti-image embeds is they arent actually supported.

Theyre a remnant of discord adding a custom feature for twitter. If you send a message with multiple embeded images, all with the same url - the discord client merges them into one, In actual data, there are still mutliple embeds, it just looks like one.

Your experiment there showcases that. The library just abstracts the attributes required to make the client merge embeds

@eightween eightween added on hold This issue/PR is on hold pending higher priority issues/PRs bug Something isn't working labels Dec 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working on hold This issue/PR is on hold pending higher priority issues/PRs
Projects
None yet
Development

No branches or pull requests

5 participants